Skip to content

Commit dfa7042

Browse files
tersalrestyled-commits
tersal
and
restyled-commits
authored
Add migration from SafeAttribute for BooleanState (project-chip#71620)
* Add migration from SafeAttribute for BooleanState * Restyled by clang-format * Address PR comments * Fix submodule changes * Restyled by clang-format * Add validation for nullptr --------- Co-authored-by: Restyled.io <commits@restyled.io>
1 parent 91334b7 commit dfa7042

9 files changed

Lines changed: 124 additions & 38 deletions

‎scripts/attribute_persistence_provider_exclusions.txt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,12 @@ src/app/clusters/chime-server/MigrateChimeServerStorage.h
3636
src/app/clusters/unit-localization-server/CodegenIntegration.cpp
3737
src/app/clusters/unit-localization-server/MigrateUnitLocalizationServerStorage.cpp
3838
src/app/clusters/unit-localization-server/MigrateUnitLocalizationServerStorage.h
39+
# BooleanStateConfiguration migration
40+
src/app/clusters/boolean-state-configuration-server/CodegenIntegration.cpp
41+
src/app/clusters/boolean-state-configuration-server/MigrateBooleanStateConfigurationStorage.cpp
42+
src/app/clusters/boolean-state-configuration-server/MigrateBooleanStateConfigurationStorage.h
3943

4044
# Cluster server implementations that currently use SafeAttributePersistenceProvider
41-
src/app/clusters/boolean-state-configuration-server/BooleanStateConfigurationCluster.cpp
42-
src/app/clusters/boolean-state-configuration-server/tests/TestBooleanStateConfigurationCluster.cpp
4345
src/app/clusters/camera-av-stream-management-server/CameraAVStreamManagementCluster.h
4446
src/app/clusters/mode-base-server/mode-base-server.cpp
4547
src/app/clusters/resource-monitoring-server/ResourceMonitoringCluster.cpp

‎src/app/clusters/boolean-state-configuration-server/BUILD.gn‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ source_set("boolean-state-configuration-server") {
2323

2424
public_deps = [
2525
"${chip_root}/src/app:attribute-persistence",
26+
"${chip_root}/src/app/persistence:migration",
2627
"${chip_root}/src/app/server-cluster",
2728
"${chip_root}/zzz_generated/app-common/clusters/BooleanStateConfiguration",
2829
]

‎src/app/clusters/boolean-state-configuration-server/BooleanStateConfigurationCluster.cpp‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
*/
1717
#include <app/clusters/boolean-state-configuration-server/BooleanStateConfigurationCluster.h>
1818

19-
#include <app/SafeAttributePersistenceProvider.h>
2019
#include <app/data-model/Decode.h>
2120
#include <app/persistence/AttributePersistence.h>
2221
#include <app/server-cluster/AttributeListBuilder.h>
@@ -93,20 +92,15 @@ CHIP_ERROR BooleanStateConfigurationCluster::Startup(ServerClusterContext & cont
9392
{
9493
ReturnErrorOnFailure(DefaultServerCluster::Startup(context));
9594

96-
if (GetSafeAttributePersistenceProvider()->ReadScalarValue({ mPath.mEndpointId, mPath.mClusterId, CurrentSensitivityLevel::Id },
97-
mCurrentSensitivityLevel) != CHIP_NO_ERROR)
98-
{
99-
mCurrentSensitivityLevel = mDefaultSensitivityLevel;
100-
}
95+
AttributePersistence attributePersistence(context.attributeStorage);
96+
97+
attributePersistence.LoadNativeEndianValue({ mPath.mEndpointId, mPath.mClusterId, CurrentSensitivityLevel::Id },
98+
mCurrentSensitivityLevel, mDefaultSensitivityLevel);
10199

102100
if (mCurrentSensitivityLevel >= mSupportedSensitivityLevels)
103101
{
104102
mCurrentSensitivityLevel = mSupportedSensitivityLevels - 1;
105103
}
106-
107-
// alarms enabled persistence was handled by ember previously (as opposed to AAI usage of sensitivity level)
108-
// TODO: this is VERY inconvenient/strange and we should really fix this inconsistence
109-
AttributePersistence attributePersistence(context.attributeStorage);
110104
AlarmModeBitMask::IntegerType alarmsEnabled;
111105
attributePersistence.LoadNativeEndianValue({ mPath.mEndpointId, mPath.mClusterId, AlarmsEnabled::Id }, alarmsEnabled,
112106
AlarmModeBitMask::IntegerType(0));
@@ -325,6 +319,7 @@ void BooleanStateConfigurationCluster::GenerateSensorFault(SensorFaultBitMask fa
325319

326320
CHIP_ERROR BooleanStateConfigurationCluster::SetCurrentSensitivityLevel(uint8_t level)
327321
{
322+
VerifyOrReturnError(mContext != nullptr, CHIP_ERROR_INCORRECT_STATE);
328323
VerifyOrReturnError(level < mSupportedSensitivityLevels, CHIP_IM_GLOBAL_STATUS(ConstraintError));
329324
VerifyOrReturnError(mCurrentSensitivityLevel != level, CHIP_NO_ERROR);
330325

@@ -335,10 +330,8 @@ CHIP_ERROR BooleanStateConfigurationCluster::SetCurrentSensitivityLevel(uint8_t
335330
mCurrentSensitivityLevel = level;
336331
NotifyAttributeChanged(CurrentSensitivityLevel::Id);
337332

338-
// TODO: we should migrate this to not use `Safe` attribute persistence and use
339-
// a common persistence layer.
340-
return GetSafeAttributePersistenceProvider()->WriteScalarValue(
341-
{ mPath.mEndpointId, mPath.mClusterId, CurrentSensitivityLevel::Id }, level);
333+
return mContext->attributeStorage.WriteValue({ mPath.mEndpointId, mPath.mClusterId, CurrentSensitivityLevel::Id },
334+
{ &mCurrentSensitivityLevel, sizeof(mCurrentSensitivityLevel) });
342335
}
343336

344337
Status BooleanStateConfigurationCluster::SetAlarmsActive(AlarmModeBitMask alarms)

‎src/app/clusters/boolean-state-configuration-server/CodegenIntegration.cpp‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
* limitations under the License.
1616
*/
1717
#include <app-common/zap-generated/attributes/Accessors.h>
18+
#include <app/SafeAttributePersistenceProvider.h>
1819
#include <app/clusters/boolean-state-configuration-server/BooleanStateConfigurationCluster.h>
20+
#include <app/clusters/boolean-state-configuration-server/MigrateBooleanStateConfigurationStorage.h>
1921
#include <app/static-cluster-config/BooleanStateConfiguration.h>
2022
#include <app/util/attribute-storage.h>
2123
#include <app/util/endpoint-config-api.h>
@@ -31,12 +33,36 @@ using namespace chip::Protocols::InteractionModel;
3133
using namespace chip::app::Clusters::BooleanStateConfiguration::Attributes;
3234
namespace {
3335

36+
/**
37+
* A BooleanStateConfigurationCluster subclass that performs storage migration during Startup.
38+
* This ensures the persistence providers are available when migration runs.
39+
*/
40+
class CodegenBooleanStateConfigurationCluster : public chip::app::Clusters::BooleanStateConfigurationCluster
41+
{
42+
public:
43+
using BooleanStateConfigurationCluster::BooleanStateConfigurationCluster;
44+
45+
CHIP_ERROR Startup(chip::app::ServerClusterContext & context) override
46+
{
47+
chip::app::SafeAttributePersistenceProvider * srcProvider = chip::app::GetSafeAttributePersistenceProvider();
48+
chip::app::AttributePersistenceProvider & dstProvider = context.attributeStorage;
49+
50+
if (srcProvider != nullptr)
51+
{
52+
LogErrorOnFailure(
53+
BooleanStateConfiguration::MigrateBooleanStateConfigurationStorage(mPath.mEndpointId, *srcProvider, dstProvider));
54+
}
55+
56+
return BooleanStateConfigurationCluster::Startup(context);
57+
}
58+
};
59+
3460
constexpr size_t kBooleanStateConfigurationFixedClusterCount =
3561
BooleanStateConfiguration::StaticApplicationConfig::kFixedClusterConfig.size();
3662
constexpr size_t kBooleanStateConfigurationMaxClusterCount =
3763
kBooleanStateConfigurationFixedClusterCount + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT;
3864

39-
LazyRegisteredServerCluster<BooleanStateConfigurationCluster> gServers[kBooleanStateConfigurationMaxClusterCount];
65+
LazyRegisteredServerCluster<CodegenBooleanStateConfigurationCluster> gServers[kBooleanStateConfigurationMaxClusterCount];
4066

4167
class IntegrationDelegate : public CodegenClusterIntegration::Delegate
4268
{

‎src/app/clusters/boolean-state-configuration-server/MigrateBooleanStateConfigurationStorage.cpp‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
*
3+
* Copyright (c) 2026 Project CHIP Authors
4+
* All rights reserved.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
#include <app/clusters/boolean-state-configuration-server/MigrateBooleanStateConfigurationStorage.h>
20+
#include <clusters/BooleanStateConfiguration/Attributes.h>
21+
#include <clusters/BooleanStateConfiguration/ClusterId.h>
22+
23+
namespace chip {
24+
namespace app {
25+
namespace Clusters {
26+
namespace BooleanStateConfiguration {
27+
28+
CHIP_ERROR MigrateBooleanStateConfigurationStorage(EndpointId endpointId, SafeAttributePersistenceProvider & safeProvider,
29+
AttributePersistenceProvider & dstProvider)
30+
{
31+
static constexpr AttrMigrationData attributesToUpdate[] = {
32+
{ Attributes::CurrentSensitivityLevel::Id, sizeof(uint8_t), true /* isScalar */ },
33+
};
34+
// We need to provide a buffer with enough space for the attributes that will be migrated.
35+
static constexpr size_t kBufferSize = MaxAttrMigrationValueSize(attributesToUpdate);
36+
static_assert(kBufferSize > 0, "All migration attributes have zero valueSize");
37+
uint8_t attributeBuffer[kBufferSize] = {};
38+
MutableByteSpan buffer(attributeBuffer);
39+
return MigrateFromSafeToAttributePersistenceProvider(safeProvider, dstProvider, { endpointId, BooleanStateConfiguration::Id },
40+
Span(attributesToUpdate), buffer);
41+
}
42+
43+
} // namespace BooleanStateConfiguration
44+
} // namespace Clusters
45+
} // namespace app
46+
} // namespace chip

‎src/app/clusters/boolean-state-configuration-server/MigrateBooleanStateConfigurationStorage.h‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
*
3+
* Copyright (c) 2026 Project CHIP Authors
4+
* All rights reserved.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
#pragma once
20+
21+
#include <app/persistence/AttributePersistenceMigration.h>
22+
#include <app/util/basic-types.h>
23+
24+
namespace chip {
25+
namespace app {
26+
namespace Clusters {
27+
namespace BooleanStateConfiguration {
28+
29+
CHIP_ERROR MigrateBooleanStateConfigurationStorage(EndpointId endpointId, SafeAttributePersistenceProvider & safeProvider,
30+
AttributePersistenceProvider & dstProvider);
31+
32+
} // namespace BooleanStateConfiguration
33+
} // namespace Clusters
34+
} // namespace app
35+
} // namespace chip

‎src/app/clusters/boolean-state-configuration-server/app_config_dependent_sources.cmake‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ TARGET_SOURCES(
1818
PRIVATE
1919
"${CLUSTER_DIR}/CodegenIntegration.cpp"
2020
"${CLUSTER_DIR}/CodegenIntegration.h"
21+
"${CLUSTER_DIR}/MigrateBooleanStateConfigurationStorage.cpp"
22+
"${CLUSTER_DIR}/MigrateBooleanStateConfigurationStorage.h"
2123
)
2224

2325
# These are the things that BUILD.gn dependencies would pull

‎src/app/clusters/boolean-state-configuration-server/app_config_dependent_sources.gni‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,7 @@ app_config_dependent_sources = [
1616
"BooleanStateConfigurationTestEventTriggerHandler.h",
1717
"CodegenIntegration.cpp",
1818
"CodegenIntegration.h",
19+
"MigrateBooleanStateConfigurationStorage.cpp",
20+
"MigrateBooleanStateConfigurationStorage.h",
1921
"boolean-state-configuration-server.h",
2022
]

‎src/app/clusters/boolean-state-configuration-server/tests/TestBooleanStateConfigurationCluster.cpp‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
*/
1616
#include <pw_unit_test/framework.h>
1717

18-
#include <app/DefaultSafeAttributePersistenceProvider.h>
19-
#include <app/SafeAttributePersistenceProvider.h>
2018
#include <app/clusters/boolean-state-configuration-server/BooleanStateConfigurationCluster.h>
2119
#include <app/clusters/boolean-state-configuration-server/boolean-state-configuration-delegate.h>
2220
#include <app/server-cluster/testing/AttributeTesting.h>
@@ -98,21 +96,6 @@ StartupConfigurationBuilder DefaultConfig()
9896
return {};
9997
}
10098

101-
class ScopedSafeAttributePersistence
102-
{
103-
public:
104-
ScopedSafeAttributePersistence(TestServerClusterContext & context) : mOldPersistence(app::GetSafeAttributePersistenceProvider())
105-
{
106-
VerifyOrDie(mPersistence.Init(&context.StorageDelegate()) == CHIP_NO_ERROR);
107-
app::SetSafeAttributePersistenceProvider(&mPersistence);
108-
}
109-
~ScopedSafeAttributePersistence() { app::SetSafeAttributePersistenceProvider(mOldPersistence); }
110-
111-
private:
112-
app::SafeAttributePersistenceProvider * mOldPersistence;
113-
app::DefaultSafeAttributePersistenceProvider mPersistence;
114-
};
115-
11699
TEST_F(TestBooleanStateConfigurationCluster, TestAttributeList)
117100
{
118101
// cluster without any attributes
@@ -262,7 +245,6 @@ TEST_F(TestBooleanStateConfigurationCluster, TestFeatureMap)
262245
TEST_F(TestBooleanStateConfigurationCluster, TestSensitivityClamping)
263246
{
264247
TestServerClusterContext context;
265-
ScopedSafeAttributePersistence persistence(context);
266248

267249
// supportedSensitivityLevels is clamped to [2, 10]
268250
{
@@ -333,7 +315,6 @@ TEST_F(TestBooleanStateConfigurationCluster, TestSensitivityClamping)
333315
TEST_F(TestBooleanStateConfigurationCluster, TestPersistenceAndStartup)
334316
{
335317
TestServerClusterContext context;
336-
ScopedSafeAttributePersistence persistence(context);
337318

338319
// 1. Create a cluster, write a value.
339320
{
@@ -408,7 +389,6 @@ TEST_F(TestBooleanStateConfigurationCluster, TestPersistenceAndStartup)
408389
TEST_F(TestBooleanStateConfigurationCluster, TestAlarmsEnabledPersistence)
409390
{
410391
TestServerClusterContext context;
411-
ScopedSafeAttributePersistence persistence(context);
412392

413393
// 1. Create a cluster, set a value for AlarmsEnabled, which should be persisted.
414394
{
@@ -579,7 +559,6 @@ class TestDelegate : public BooleanStateConfiguration::Delegate
579559
TEST_F(TestBooleanStateConfigurationCluster, TestTypeSafeDelegateCallbacks)
580560
{
581561
TestServerClusterContext context;
582-
ScopedSafeAttributePersistence persistence(context);
583562
TestDelegate delegate;
584563

585564
// Test CurrentSensitivityLevel callback via WriteAttribute

0 commit comments

Comments
 (0)