Skip to content

Commit 2f01962

Browse files
lpbeliveau-silabsbzbarsky-apple
authored and
shgutte
committed
[Scenes] Missing Tests Cases (project-chip#32632)
* Implemented missing yaml tests cases and added EFS validation in SceneHandlerImpl * Apply suggestions from code review Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Cleanup up comments and useless static cast --------- Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
1 parent 7ada478 commit 2f01962

File tree

8 files changed

+1067
-55
lines changed

8 files changed

+1067
-55
lines changed

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

+242-1
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,249 @@
1616
*/
1717

1818
#include <app/clusters/scenes-server/SceneHandlerImpl.h>
19+
#include <app/util/ember-io-storage.h>
20+
#include <app/util/endpoint-config-api.h>
21+
#include <app/util/odd-sized-integers.h>
1922

2023
namespace chip {
2124
namespace scenes {
2225

26+
namespace {
27+
28+
template <int ByteSize, bool IsSigned>
29+
using OddSizedInteger = app::OddSizedInteger<ByteSize, IsSigned>;
30+
using ConcreteAttributePath = app::ConcreteAttributePath;
31+
using AttributeValuePairType = app::Clusters::ScenesManagement::Structs::AttributeValuePairStruct::Type;
32+
33+
/// ConvertDefaultValueToWorkingValue
34+
/// @brief Helper function to convert a byte array to a value of the given type.
35+
/// @param EmberAfDefaultAttributeValue & defaultValue
36+
/// @return Value converted to the given working type
37+
template <typename Type>
38+
typename app::NumericAttributeTraits<Type>::WorkingType
39+
ConvertDefaultValueToWorkingValue(const EmberAfDefaultAttributeValue & defaultValue)
40+
{
41+
if constexpr (sizeof(typename app::NumericAttributeTraits<Type>::WorkingType) <= 2)
42+
{
43+
return static_cast<typename app::NumericAttributeTraits<Type>::WorkingType>(defaultValue.defaultValue);
44+
}
45+
46+
typename app::NumericAttributeTraits<Type>::StorageType sValue;
47+
memcpy(&sValue, defaultValue.ptrToDefaultValue, sizeof(sValue));
48+
return app::NumericAttributeTraits<Type>::StorageToWorking(sValue);
49+
}
50+
51+
/// IsExactlyOneValuePopulated
52+
/// @brief Helper function to verify that exactly one value is populated in a given AttributeValuePairType
53+
/// @param AttributeValuePairType & type AttributeValuePairType to verify
54+
/// @return bool true if only one value is populated, false otherwise
55+
bool IsExactlyOneValuePopulated(const AttributeValuePairType & type)
56+
{
57+
int count = 0;
58+
if (type.valueUnsigned8.HasValue())
59+
count++;
60+
if (type.valueSigned8.HasValue())
61+
count++;
62+
if (type.valueUnsigned16.HasValue())
63+
count++;
64+
if (type.valueSigned16.HasValue())
65+
count++;
66+
if (type.valueUnsigned32.HasValue())
67+
count++;
68+
if (type.valueSigned32.HasValue())
69+
count++;
70+
if (type.valueUnsigned64.HasValue())
71+
count++;
72+
if (type.valueSigned64.HasValue())
73+
count++;
74+
return count == 1;
75+
}
76+
77+
/// CapAttributeValue
78+
/// Cap the attribute value based on the attribute's min and max if they are defined,
79+
/// or based on the attribute's size if they are not.
80+
/// @param[in] aVPair AttributeValuePairType
81+
/// @param[in] metadata EmberAfAttributeMetadata
82+
///
83+
template <typename Type>
84+
void CapAttributeValue(typename app::NumericAttributeTraits<Type>::WorkingType & value, const EmberAfAttributeMetadata * metadata)
85+
{
86+
using IntType = app::NumericAttributeTraits<Type>;
87+
using WorkingType = typename IntType::WorkingType;
88+
89+
WorkingType maxValue;
90+
WorkingType minValue;
91+
uint16_t bitWidth = static_cast<uint16_t>(emberAfAttributeSize(metadata) * 8);
92+
93+
// TODO Use min/max values from Type to obtain min/max instead of relying on metadata. See:
94+
// https://github.com/project-chip/connectedhomeip/issues/35328
95+
96+
// Min/Max Value caps for the OddSize integers
97+
if (metadata->IsSignedIntegerAttribute())
98+
{
99+
// We use emberAfAttributeSize for cases like INT24S, INT40S, INT48S, INT56S where numeric_limits<WorkingType>::max()
100+
// wouldn't work
101+
maxValue = static_cast<WorkingType>((1ULL << (bitWidth - 1)) - 1);
102+
minValue = static_cast<WorkingType>(-(1ULL << (bitWidth - 1)));
103+
}
104+
else
105+
{
106+
// We use emberAfAttributeSize for cases like INT24U, INT40U, INT48U, INT56U where numeric_limits<WorkingType>::max()
107+
// wouldn't work
108+
if (ZCL_INT64U_ATTRIBUTE_TYPE == app::Compatibility::Internal::AttributeBaseType(metadata->attributeType))
109+
{
110+
maxValue = static_cast<WorkingType>(UINT64_MAX); // Bit shift of 64 is undefined so we use UINT64_MAX
111+
}
112+
else
113+
{
114+
maxValue = static_cast<WorkingType>((1ULL << bitWidth) - 1);
115+
}
116+
minValue = static_cast<WorkingType>(0);
117+
}
118+
119+
// Ensure that the metadata's signedness matches the working type's signedness
120+
VerifyOrDie(metadata->IsSignedIntegerAttribute() == std::is_signed<WorkingType>::value);
121+
122+
if (metadata->IsBoolean())
123+
{
124+
if (metadata->IsNullable() && (value != 1 && value != 0))
125+
{
126+
// If the attribute is nullable, the value can be set to NULL
127+
app::NumericAttributeTraits<WorkingType>::SetNull(value);
128+
}
129+
else
130+
{
131+
// Caping the value to 1 in case values greater than 1 are set
132+
value = value ? 1 : 0;
133+
}
134+
return;
135+
}
136+
137+
// Check metadata for min and max values
138+
if (metadata->HasMinMax())
139+
{
140+
const EmberAfAttributeMinMaxValue * minMaxValue = metadata->defaultValue.ptrToMinMaxValue;
141+
minValue = ConvertDefaultValueToWorkingValue<Type>(minMaxValue->minValue);
142+
maxValue = ConvertDefaultValueToWorkingValue<Type>(minMaxValue->maxValue);
143+
}
144+
145+
// If the attribute is nullable, the min and max values calculated for types will not be valid, however this does not
146+
// change the behavior here as the value will already be NULL if it is out of range. E.g. a nullable INT8U has a minValue of
147+
// -127. The code above determin minValue = -128, so an input value of -128 would not enter the condition block below, but would
148+
// be considered NULL nonetheless.
149+
if (metadata->IsNullable() && (minValue > value || maxValue < value))
150+
{
151+
// If the attribute is nullable, the value can be set to NULL
152+
app::NumericAttributeTraits<WorkingType>::SetNull(value);
153+
return;
154+
}
155+
156+
if (minValue > value)
157+
{
158+
value = minValue;
159+
}
160+
else if (maxValue < value)
161+
{
162+
value = maxValue;
163+
}
164+
}
165+
166+
/// @brief Validate the attribute exists for a given cluster
167+
/// @param[in] endpoint Endpoint ID
168+
/// @param[in] clusterID Cluster ID
169+
/// @param[in] aVPair AttributeValuePairType, will be mutated to cap the value if it is out of range
170+
/// @return CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute) if the attribute does not exist for a given cluster or is not scenable
171+
/// @note This will allways fail for global list attributes. If we do want to make them scenable someday, we will need to
172+
/// use a different validation method.
173+
// TODO: Add check for "S" quality to determine if the attribute is scenable once suported :
174+
// https://github.com/project-chip/connectedhomeip/issues/24177
175+
CHIP_ERROR ValidateAttributePath(EndpointId endpoint, ClusterId cluster, AttributeValuePairType & aVPair)
176+
{
177+
const EmberAfAttributeMetadata * metadata = emberAfLocateAttributeMetadata(endpoint, cluster, aVPair.attributeID);
178+
179+
if (nullptr == metadata)
180+
{
181+
return CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute);
182+
}
183+
184+
// There should never be more than one populated value in an ExtensionFieldSet
185+
VerifyOrReturnError(IsExactlyOneValuePopulated(aVPair), CHIP_ERROR_INVALID_ARGUMENT);
186+
187+
switch (app::Compatibility::Internal::AttributeBaseType(metadata->attributeType))
188+
{
189+
case ZCL_BOOLEAN_ATTRIBUTE_TYPE:
190+
case ZCL_INT8U_ATTRIBUTE_TYPE:
191+
VerifyOrReturnError(aVPair.valueUnsigned8.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
192+
CapAttributeValue<uint8_t>(aVPair.valueUnsigned8.Value(), metadata);
193+
break;
194+
case ZCL_INT16U_ATTRIBUTE_TYPE:
195+
VerifyOrReturnError(aVPair.valueUnsigned16.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
196+
CapAttributeValue<uint16_t>(aVPair.valueUnsigned16.Value(), metadata);
197+
break;
198+
case ZCL_INT24U_ATTRIBUTE_TYPE:
199+
VerifyOrReturnError(aVPair.valueUnsigned32.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
200+
CapAttributeValue<OddSizedInteger<3, false>>(aVPair.valueUnsigned32.Value(), metadata);
201+
break;
202+
case ZCL_INT32U_ATTRIBUTE_TYPE:
203+
VerifyOrReturnError(aVPair.valueUnsigned32.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
204+
CapAttributeValue<uint32_t>(aVPair.valueUnsigned32.Value(), metadata);
205+
break;
206+
case ZCL_INT40U_ATTRIBUTE_TYPE:
207+
VerifyOrReturnError(aVPair.valueUnsigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
208+
CapAttributeValue<OddSizedInteger<5, false>>(aVPair.valueUnsigned64.Value(), metadata);
209+
break;
210+
case ZCL_INT48U_ATTRIBUTE_TYPE:
211+
VerifyOrReturnError(aVPair.valueUnsigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
212+
CapAttributeValue<OddSizedInteger<6, false>>(aVPair.valueUnsigned64.Value(), metadata);
213+
break;
214+
case ZCL_INT56U_ATTRIBUTE_TYPE:
215+
VerifyOrReturnError(aVPair.valueUnsigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
216+
CapAttributeValue<OddSizedInteger<7, false>>(aVPair.valueUnsigned64.Value(), metadata);
217+
break;
218+
case ZCL_INT64U_ATTRIBUTE_TYPE:
219+
VerifyOrReturnError(aVPair.valueUnsigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
220+
CapAttributeValue<uint64_t>(aVPair.valueUnsigned64.Value(), metadata);
221+
break;
222+
case ZCL_INT8S_ATTRIBUTE_TYPE:
223+
VerifyOrReturnError(aVPair.valueSigned8.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
224+
CapAttributeValue<int8_t>(aVPair.valueSigned8.Value(), metadata);
225+
break;
226+
case ZCL_INT16S_ATTRIBUTE_TYPE:
227+
VerifyOrReturnError(aVPair.valueSigned16.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
228+
CapAttributeValue<int16_t>(aVPair.valueSigned16.Value(), metadata);
229+
break;
230+
case ZCL_INT24S_ATTRIBUTE_TYPE:
231+
VerifyOrReturnError(aVPair.valueSigned32.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
232+
CapAttributeValue<OddSizedInteger<3, true>>(aVPair.valueSigned32.Value(), metadata);
233+
break;
234+
case ZCL_INT32S_ATTRIBUTE_TYPE:
235+
VerifyOrReturnError(aVPair.valueSigned32.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
236+
CapAttributeValue<int32_t>(aVPair.valueSigned32.Value(), metadata);
237+
break;
238+
case ZCL_INT40S_ATTRIBUTE_TYPE:
239+
VerifyOrReturnError(aVPair.valueSigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
240+
CapAttributeValue<OddSizedInteger<5, true>>(aVPair.valueSigned64.Value(), metadata);
241+
break;
242+
case ZCL_INT48S_ATTRIBUTE_TYPE:
243+
VerifyOrReturnError(aVPair.valueSigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
244+
CapAttributeValue<OddSizedInteger<6, true>>(aVPair.valueSigned64.Value(), metadata);
245+
break;
246+
case ZCL_INT56S_ATTRIBUTE_TYPE:
247+
VerifyOrReturnError(aVPair.valueSigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
248+
CapAttributeValue<OddSizedInteger<7, true>>(aVPair.valueSigned64.Value(), metadata);
249+
break;
250+
case ZCL_INT64S_ATTRIBUTE_TYPE:
251+
VerifyOrReturnError(aVPair.valueSigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
252+
CapAttributeValue<int64_t>(aVPair.valueSigned64.Value(), metadata);
253+
break;
254+
default:
255+
return CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute);
256+
}
257+
258+
return CHIP_NO_ERROR;
259+
}
260+
} // namespace
261+
23262
CHIP_ERROR
24263
DefaultSceneHandlerImpl::EncodeAttributeValueList(const List<AttributeValuePairType> & aVlist, MutableByteSpan & serializedBytes)
25264
{
@@ -58,7 +297,9 @@ DefaultSceneHandlerImpl::SerializeAdd(EndpointId endpoint, const ExtensionFieldS
58297
auto pair_iterator = extensionFieldSet.attributeValueList.begin();
59298
while (pair_iterator.Next())
60299
{
61-
aVPairs[pairCount] = pair_iterator.GetValue();
300+
AttributeValuePairType currentPair = pair_iterator.GetValue();
301+
ReturnErrorOnFailure(ValidateAttributePath(endpoint, extensionFieldSet.clusterID, currentPair));
302+
aVPairs[pairCount] = currentPair;
62303
pairCount++;
63304
}
64305
ReturnErrorOnFailure(pair_iterator.GetStatus());

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

+5
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ CHIP_ERROR AddResponseOnError(CommandHandlerInterface::HandlerContext & ctx, Res
7474
{
7575
resp.status = to_underlying(Protocols::InteractionModel::Status::ResourceExhausted);
7676
}
77+
else if (CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute) == err)
78+
{
79+
// TODO: Confirm if we need to add UnsupportedAttribute status as a return for Scene Commands
80+
resp.status = to_underlying(Protocols::InteractionModel::Status::InvalidCommand);
81+
}
7782
else
7883
{
7984
resp.status = to_underlying(StatusIB(err).mStatus);

0 commit comments

Comments
 (0)