Skip to content

Commit ef676ad

Browse files
lpbeliveau-silabslpbeliveau-silabs
committed
Added check on maximal attribute value when executing add scenes, along with unit tests and refactor of AttributeValuePair->Value to uint64
1 parent ef0ac43 commit ef676ad

File tree

24 files changed

+113
-49
lines changed

24 files changed

+113
-49
lines changed

‎docs/ERROR_CODES.md

+1
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ This file was **AUTOMATICALLY** generated by
8989
| 92 | 0x5C | `CHIP_ERROR_WRONG_NODE_ID` |
9090
| 100 | 0x64 | `CHIP_ERROR_RETRANS_TABLE_FULL` |
9191
| 104 | 0x68 | `CHIP_ERROR_TRANSACTION_CANCELED` |
92+
| 106 | 0x6A | `CHIP_ERROR_UNSUPPORTED_ATTRIBUTE` |
9293
| 107 | 0x6B | `CHIP_ERROR_INVALID_SUBSCRIPTION` |
9394
| 108 | 0x6C | `CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE` |
9495
| 112 | 0x70 | `CHIP_ERROR_UNSOLICITED_MSG_NO_ORIGINATOR` |

‎examples/all-clusters-app/all-clusters-common/all-clusters-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -3514,7 +3514,7 @@ provisional cluster ScenesManagement = 98 {
35143514

35153515
struct AttributeValuePair {
35163516
attrib_id attributeID = 0;
3517-
int32u attributeValue = 1;
3517+
int64u attributeValue = 1;
35183518
}
35193519

35203520
struct ExtensionFieldSet {

‎examples/all-clusters-minimal-app/all-clusters-common/all-clusters-minimal-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -2426,7 +2426,7 @@ provisional cluster ScenesManagement = 98 {
24262426

24272427
struct AttributeValuePair {
24282428
attrib_id attributeID = 0;
2429-
int32u attributeValue = 1;
2429+
int64u attributeValue = 1;
24302430
}
24312431

24322432
struct ExtensionFieldSet {

‎examples/light-switch-app/light-switch-common/light-switch-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -2037,7 +2037,7 @@ provisional cluster ScenesManagement = 98 {
20372037

20382038
struct AttributeValuePair {
20392039
attrib_id attributeID = 0;
2040-
int32u attributeValue = 1;
2040+
int64u attributeValue = 1;
20412041
}
20422042

20432043
struct ExtensionFieldSet {

‎examples/light-switch-app/qpg/zap/switch.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1834,7 +1834,7 @@ provisional cluster ScenesManagement = 98 {
18341834

18351835
struct AttributeValuePair {
18361836
attrib_id attributeID = 0;
1837-
int32u attributeValue = 1;
1837+
int64u attributeValue = 1;
18381838
}
18391839

18401840
struct ExtensionFieldSet {

‎examples/lighting-app/lighting-common/lighting-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1854,7 +1854,7 @@ provisional cluster ScenesManagement = 98 {
18541854

18551855
struct AttributeValuePair {
18561856
attrib_id attributeID = 0;
1857-
int32u attributeValue = 1;
1857+
int64u attributeValue = 1;
18581858
}
18591859

18601860
struct ExtensionFieldSet {

‎examples/lighting-app/silabs/data_model/lighting-thread-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1559,7 +1559,7 @@ provisional cluster ScenesManagement = 98 {
15591559

15601560
struct AttributeValuePair {
15611561
attrib_id attributeID = 0;
1562-
int32u attributeValue = 1;
1562+
int64u attributeValue = 1;
15631563
}
15641564

15651565
struct ExtensionFieldSet {

‎examples/lighting-app/silabs/data_model/lighting-wifi-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1850,7 +1850,7 @@ provisional cluster ScenesManagement = 98 {
18501850

18511851
struct AttributeValuePair {
18521852
attrib_id attributeID = 0;
1853-
int32u attributeValue = 1;
1853+
int64u attributeValue = 1;
18541854
}
18551855

18561856
struct ExtensionFieldSet {

‎examples/placeholder/linux/apps/app1/config.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -3020,7 +3020,7 @@ provisional cluster ScenesManagement = 98 {
30203020

30213021
struct AttributeValuePair {
30223022
attrib_id attributeID = 0;
3023-
int32u attributeValue = 1;
3023+
int64u attributeValue = 1;
30243024
}
30253025

30263026
struct ExtensionFieldSet {

‎examples/placeholder/linux/apps/app2/config.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -2977,7 +2977,7 @@ provisional cluster ScenesManagement = 98 {
29772977

29782978
struct AttributeValuePair {
29792979
attrib_id attributeID = 0;
2980-
int32u attributeValue = 1;
2980+
int64u attributeValue = 1;
29812981
}
29822982

29832983
struct ExtensionFieldSet {

‎examples/thermostat/nxp/zap/thermostat_matter_thread.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1755,7 +1755,7 @@ provisional cluster ScenesManagement = 98 {
17551755

17561756
struct AttributeValuePair {
17571757
attrib_id attributeID = 0;
1758-
int32u attributeValue = 1;
1758+
int64u attributeValue = 1;
17591759
}
17601760

17611761
struct ExtensionFieldSet {

‎examples/thermostat/nxp/zap/thermostat_matter_wifi.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1666,7 +1666,7 @@ provisional cluster ScenesManagement = 98 {
16661666

16671667
struct AttributeValuePair {
16681668
attrib_id attributeID = 0;
1669-
int32u attributeValue = 1;
1669+
int64u attributeValue = 1;
16701670
}
16711671

16721672
struct ExtensionFieldSet {

‎examples/virtual-device-app/virtual-device-common/virtual-device-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1902,7 +1902,7 @@ provisional cluster ScenesManagement = 98 {
19021902

19031903
struct AttributeValuePair {
19041904
attrib_id attributeID = 0;
1905-
int32u attributeValue = 1;
1905+
int64u attributeValue = 1;
19061906
}
19071907

19081908
struct ExtensionFieldSet {

‎src/app/clusters/level-control/level-control.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ class DefaultLevelControlSceneHandler : public scenes::DefaultSceneHandlerImpl
177177
}
178178
else
179179
{
180-
chip::app::NumericAttributeTraits<uint32_t>::SetNull(pairs[0].attributeValue);
180+
chip::app::NumericAttributeTraits<uint64_t>::SetNull(pairs[0].attributeValue);
181181
}
182182
size_t attributeCount = 1;
183183
if (LevelControlHasFeature(endpoint, LevelControl::Feature::kFrequency))

‎src/app/clusters/scenes-server/SceneHandlerImpl.cpp

+35-7
Original file line numberDiff line numberDiff line change
@@ -16,29 +16,56 @@
1616
*/
1717

1818
#include <app/clusters/scenes-server/SceneHandlerImpl.h>
19+
#include <app/util/endpoint-config-api.h>
1920

2021
namespace chip {
2122
namespace scenes {
2223

2324
namespace {
2425

25-
/// ValideAttribute
26+
using ConcreteAttributePath = app::ConcreteAttributePath;
27+
using AttributeValuePairType = app::Clusters::ScenesManagement::Structs::AttributeValuePair::Type;
28+
29+
/// CapAttributeID
30+
/// Cap the attribute value based on the attribute type size
31+
/// @param[in] aVPair AttributeValuePairType
32+
/// @param[in] metadata EmberAfAttributeMetadata
33+
///
34+
void CapAttributeID(AttributeValuePairType & aVPair, const EmberAfAttributeMetadata * metadata)
35+
{
36+
// Calculate the maximum value that can be represented with the given number of bytes
37+
uint64_t maxValue = (1ULL << (metadata->size * 8)) - 1;
38+
39+
// If the attribute ID is greater than the maximum value, cap it
40+
if (aVPair.attributeValue > maxValue)
41+
{
42+
aVPair.attributeValue = maxValue;
43+
}
44+
}
45+
2646
/// @brief Validate the attribute exists for a given cluster
47+
/// @param[in] endpoint Endpoint ID
2748
/// @param[in] clusterID Cluster ID
28-
/// @param[in] attID Attribute ID
49+
/// @param[in] aVPair AttributeValuePairType, will be mutated to cap the value if it is out of range
2950
/// @return CHIP_ERROR_UNSUPPORTED_ATTRIBUTE if the attribute does not exist for a given cluster or is not scenable
3051
/// @note This will allways fail for global list attributes. If we do want to make them scenable someday, we will need to
3152
/// use a different validation method.
32-
// TODO: Assess if we also want to throw an error if the attribute value is out of range
53+
// TODO: Assess if (and how) we also want to throw an error if the attribute value is out of range
3354
// TODO: Add check for "S" quality to determine if the attribute is scenable once suported :
3455
// https://github.com/project-chip/connectedhomeip/issues/24177
35-
CHIP_ERROR ValidateAttributePath(EndpointId endpoint, ClusterId cluster, AttributeId attributeId)
56+
CHIP_ERROR ValidateAttributePath(EndpointId endpoint, ClusterId cluster, AttributeValuePairType & aVPair)
3657
{
37-
bool attIndex = emberAfContainsAttribute(endpoint, cluster, attributeId);
58+
bool attIndex = emberAfContainsAttribute(endpoint, cluster, aVPair.attributeID);
3859
if (!attIndex)
3960
{
4061
return CHIP_ERROR_UNSUPPORTED_ATTRIBUTE;
4162
}
63+
64+
EmberAfAttributeMetadata metadata = *emberAfLocateAttributeMetadata(endpoint, cluster, aVPair.attributeID);
65+
66+
// Cap value based on the attribute type size
67+
CapAttributeID(aVPair, &metadata);
68+
4269
return CHIP_NO_ERROR;
4370
}
4471
} // namespace
@@ -81,8 +108,9 @@ DefaultSceneHandlerImpl::SerializeAdd(EndpointId endpoint, const ExtensionFieldS
81108
auto pair_iterator = extensionFieldSet.attributeValueList.begin();
82109
while (pair_iterator.Next())
83110
{
84-
ReturnErrorOnFailure(ValidateAttributePath(endpoint, extensionFieldSet.clusterID, pair_iterator.GetValue().attributeID));
85-
aVPairs[pairCount] = pair_iterator.GetValue();
111+
AttributeValuePairType currentPair = pair_iterator.GetValue();
112+
ReturnErrorOnFailure(ValidateAttributePath(endpoint, extensionFieldSet.clusterID, currentPair));
113+
aVPairs[pairCount] = currentPair;
86114
pairCount++;
87115
}
88116
ReturnErrorOnFailure(pair_iterator.GetStatus());

‎src/app/tests/TestSceneTable.cpp

+38
Original file line numberDiff line numberDiff line change
@@ -722,6 +722,44 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext)
722722

723723
memset(failBuffer, 0, fail_list.size());
724724
memset(buffer, 0, buff_span.size());
725+
726+
// Test Serialize Add of an attribute value that is greater than the mock attribute size (size of uint32_t)
727+
OOPairs[0].attributeValue = 0xFFFFFFFF'FFFFFFFF;
728+
729+
// EFS to test caping of value once a variable above the mock attribute size is serialize
730+
app::Clusters::ScenesManagement::Structs::ExtensionFieldSet::Type extensionFieldValueCapOut;
731+
app::Clusters::ScenesManagement::Structs::ExtensionFieldSet::DecodableType extensionFieldValueCapIn;
732+
733+
extensionFieldValueCapOut.clusterID = kOnOffClusterId;
734+
extensionFieldValueCapOut.attributeValueList = OOPairs;
735+
736+
/// Setup of input EFS (by temporary using the output one)
737+
writer.Init(buff_span);
738+
NL_TEST_ASSERT(
739+
aSuite, CHIP_NO_ERROR == app::DataModel::Encode(writer, TLV::AnonymousTag(), extensionFieldValueCapOut.attributeValueList));
740+
741+
reader.Init(buffer);
742+
extensionFieldValueCapIn.clusterID = kOnOffClusterId;
743+
NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.Next());
744+
NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == extensionFieldValueCapIn.attributeValueList.Decode(reader));
745+
746+
// Verify that the initial value is not capped
747+
auto pair_iterator = extensionFieldValueCapIn.attributeValueList.begin();
748+
pair_iterator.Next();
749+
app::Clusters::ScenesManagement::Structs::AttributeValuePair::Type pair = pair_iterator.GetValue();
750+
NL_TEST_ASSERT(aSuite, pair.attributeValue == OOPairs[0].attributeValue);
751+
752+
// Verify that we cap the value to the mock attribute size when serializing
753+
NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sHandler.SerializeAdd(kTestEndpoint1, extensionFieldValueCapIn, buff_span));
754+
NL_TEST_ASSERT(aSuite,
755+
CHIP_NO_ERROR == sHandler.Deserialize(kTestEndpoint1, kOnOffClusterId, buff_span, extensionFieldValueCapOut));
756+
757+
// Verify that the output value is capped
758+
NL_TEST_ASSERT(aSuite, extensionFieldValueCapOut.attributeValueList[0].attributeValue == 0x00000000'FFFFFFFF);
759+
760+
// Clear buffer
761+
memset(failBuffer, 0, fail_list.size());
762+
memset(buffer, 0, buff_span.size());
725763
};
726764

727765
void TestHandlerHelpers(nlTestSuite * aSuite, void * aContext) {}

‎src/app/util/mock/Functions.h

-3
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,6 @@ void ResetVersion();
4747
/// be returned by emberAfDataVersionStorage
4848
DataVersion GetVersion();
4949

50-
/// Configures the singular global mock attribute storage to use the specified configuration.
51-
void SetMockNodeConfig(const MockNodeConfig & config);
52-
5350
/// Resets the mock attribute storage to the default configuration.
5451
void ResetMockNodeConfig();
5552

‎src/app/util/mock/attribute-storage.cpp

+18-18
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,20 @@ bool mockAttribute1 = true;
6969
int16_t mockAttribute2 = 42;
7070
uint64_t mockAttribute3 = 0xdeadbeef0000cafe;
7171
uint8_t mockAttribute4[256] = {
72-
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
73-
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
74-
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
75-
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
76-
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
77-
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
78-
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
79-
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
72+
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
73+
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
74+
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
75+
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
76+
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
77+
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
78+
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
79+
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
8080
};
81+
EmberAfAttributeMetadata mockmetadata = { .defaultValue = EmberAfDefaultOrMinMaxAttributeValue(static_cast<uint32_t>(0)),
82+
.attributeId = 0,
83+
.size = sizeof(uint32_t),
84+
.attributeType = 0,
85+
.mask = 0 }; // namespace
8186

8287
} // namespace
8388

@@ -212,6 +217,11 @@ DataVersion * emberAfDataVersionStorage(const chip::app::ConcreteClusterPath & a
212217
return &dataVersion;
213218
}
214219

220+
const EmberAfAttributeMetadata * emberAfLocateAttributeMetadata(EndpointId endpoint, ClusterId clusterId, AttributeId attributeId)
221+
{
222+
return &mockmetadata;
223+
}
224+
215225
namespace chip {
216226
namespace app {
217227

@@ -361,15 +371,5 @@ CHIP_ERROR ReadSingleMockClusterData(FabricIndex aAccessingFabricIndex, const Co
361371
return attributeReport.EndOfAttributeReportIB();
362372
}
363373

364-
void SetMockNodeConfig(const MockNodeConfig & config)
365-
{
366-
mockConfig = &config;
367-
}
368-
369-
void ResetMockNodeConfig()
370-
{
371-
mockConfig = nullptr;
372-
}
373-
374374
} // namespace Test
375375
} // namespace chip

‎src/app/zap-templates/zcl/data-model/chip/scene.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ limitations under the License.
3232
<struct name="AttributeValuePair">
3333
<cluster code="0x0062"/>
3434
<item name="AttributeID" type="attrib_id" optional="false"/>
35-
<item name="AttributeValue" type="int32u" optional="false"/>
35+
<item name="AttributeValue" type="int64u" optional="false"/>
3636
</struct>
3737

3838
<struct name="ExtensionFieldSet">

‎src/controller/data_model/controller-clusters.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -3750,7 +3750,7 @@ provisional cluster ScenesManagement = 98 {
37503750

37513751
struct AttributeValuePair {
37523752
attrib_id attributeID = 0;
3753-
int32u attributeValue = 1;
3753+
int64u attributeValue = 1;
37543754
}
37553755

37563756
struct ExtensionFieldSet {

‎src/controller/java/generated/java/matter/controller/cluster/structs/ScenesManagementClusterAttributeValuePair.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import matter.tlv.Tag
2222
import matter.tlv.TlvReader
2323
import matter.tlv.TlvWriter
2424

25-
class ScenesManagementClusterAttributeValuePair(val attributeID: UInt, val attributeValue: UInt) {
25+
class ScenesManagementClusterAttributeValuePair(val attributeID: UInt, val attributeValue: ULong) {
2626
override fun toString(): String = buildString {
2727
append("ScenesManagementClusterAttributeValuePair {\n")
2828
append("\tattributeID : $attributeID\n")
@@ -46,7 +46,7 @@ class ScenesManagementClusterAttributeValuePair(val attributeID: UInt, val attri
4646
fun fromTlv(tlvTag: Tag, tlvReader: TlvReader): ScenesManagementClusterAttributeValuePair {
4747
tlvReader.enterStructure(tlvTag)
4848
val attributeID = tlvReader.getUInt(ContextSpecificTag(TAG_ATTRIBUTE_I_D))
49-
val attributeValue = tlvReader.getUInt(ContextSpecificTag(TAG_ATTRIBUTE_VALUE))
49+
val attributeValue = tlvReader.getULong(ContextSpecificTag(TAG_ATTRIBUTE_VALUE))
5050

5151
tlvReader.exitContainer()
5252

‎src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.mm

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎zzz_generated/app-common/app-common/zap-generated/cluster-objects.h

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎zzz_generated/darwin-framework-tool/zap-generated/cluster/Commands.h

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)