Skip to content

Commit 471c5cb

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 471c5cb

File tree

23 files changed

+105
-28
lines changed

23 files changed

+105
-28
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/attribute-storage.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ uint8_t mockAttribute4[256] = {
7878
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,
7979
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

‎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)