Skip to content

Commit bc694d8

Browse files
lpbeliveau-silabsbzbarsky-apple
lpbeliveau-silabs
and
bzbarsky-apple
committed
Apply suggestions from code review
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
1 parent 90daf92 commit bc694d8

File tree

1 file changed

+44
-28
lines changed

1 file changed

+44
-28
lines changed

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

+44-28
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717

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

@@ -37,7 +38,7 @@ template <typename Type>
3738
typename app::NumericAttributeTraits<Type>::WorkingType
3839
ConvertDefaultValueToWorkingValue(const EmberAfDefaultAttributeValue & defaultValue)
3940
{
40-
if (sizeof(typename app::NumericAttributeTraits<Type>::WorkingType) <= 2)
41+
if constexpr (sizeof(typename app::NumericAttributeTraits<Type>::WorkingType) <= 2)
4142
{
4243
return static_cast<typename app::NumericAttributeTraits<Type>::WorkingType>(defaultValue.defaultValue);
4344
}
@@ -47,11 +48,11 @@ ConvertDefaultValueToWorkingValue(const EmberAfDefaultAttributeValue & defaultVa
4748
return app::NumericAttributeTraits<Type>::StorageToWorking(sValue);
4849
}
4950

50-
/// IsOnlyOneValuePopulated
51-
/// @brief Helper function to verify if only one value is populated in a given AttributeValuePairType
51+
/// IsExactlyOneValuePopulated
52+
/// @brief Helper function to verify that exactly one value is populated in a given AttributeValuePairType
5253
/// @param AttributeValuePairType & type AttributeValuePairType to verify
5354
/// @return bool true if only one value is populated, false otherwise
54-
bool IsOnlyOneValuePopulated(const AttributeValuePairType & type)
55+
bool IsExactlyOneValuePopulated(const AttributeValuePairType & type)
5556
{
5657
int count = 0;
5758
if (type.valueUnsigned8.HasValue())
@@ -73,14 +74,14 @@ bool IsOnlyOneValuePopulated(const AttributeValuePairType & type)
7374
return count == 1;
7475
}
7576

76-
/// CapAttributeID
77+
/// CapAttributeValue
7778
/// Cap the attribute value based on the attribute's min and max if they are defined,
7879
/// or based on the attribute's size if they are not.
7980
/// @param[in] aVPair AttributeValuePairType
8081
/// @param[in] metadata EmberAfAttributeMetadata
8182
///
8283
template <typename Type>
83-
void CapAttributeID(typename app::NumericAttributeTraits<Type>::WorkingType & Value, const EmberAfAttributeMetadata * metadata)
84+
void CapAttributeValue(typename app::NumericAttributeTraits<Type>::WorkingType & value, const EmberAfAttributeMetadata * metadata)
8485
{
8586
using IntType = app::NumericAttributeTraits<Type>;
8687
using WorkingType = typename IntType::WorkingType;
@@ -89,7 +90,7 @@ void CapAttributeID(typename app::NumericAttributeTraits<Type>::WorkingType & Va
8990
WorkingType minValue;
9091
uint16_t bitWidth = static_cast<uint16_t>(emberAfAttributeSize(metadata) * 8);
9192

92-
// Min/Max Value capps for the OddSize integers
93+
// Min/Max Value caps for the OddSize integers
9394
if (metadata->IsSignedIntegerAttribute())
9495
{
9596
// We use emberAfAttributeSize for cases like INT24S, INT40S, INT48S, INT56S where numeric_limits<WorkingType>::max()
@@ -118,7 +119,7 @@ void CapAttributeID(typename app::NumericAttributeTraits<Type>::WorkingType & Va
118119
if (metadata->IsBoolean())
119120
{
120121
// Caping the value to 1 in case values greater than 1 are set
121-
Value = Value ? 1 : 0;
122+
value = value ? 1 : 0;
122123
return;
123124
}
124125

@@ -130,13 +131,20 @@ void CapAttributeID(typename app::NumericAttributeTraits<Type>::WorkingType & Va
130131
maxValue = ConvertDefaultValueToWorkingValue<Type>(minMaxValue->maxValue);
131132
}
132133

133-
if (minValue > Value)
134+
if (metadata->IsNullable() && (minValue > value || maxValue < value))
134135
{
135-
Value = minValue;
136+
// If the attribute is nullable, the value can be set to NULL
137+
app::NumericAttributeTraits<WorkingType>::SetNull(value);
138+
return;
139+
}
140+
141+
if (minValue > value)
142+
{
143+
value = minValue;
136144
}
137-
else if (maxValue < Value)
145+
else if (maxValue < value)
138146
{
139-
Value = maxValue;
147+
value = maxValue;
140148
}
141149
}
142150

@@ -159,72 +167,80 @@ CHIP_ERROR ValidateAttributePath(EndpointId endpoint, ClusterId cluster, Attribu
159167
}
160168

161169
// There should never be more than one populated value in an ExtensionFieldSet
162-
VerifyOrReturnError(IsOnlyOneValuePopulated(aVPair), CHIP_ERROR_INVALID_ARGUMENT);
170+
VerifyOrReturnError(IsExactlyOneValuePopulated(aVPair), CHIP_ERROR_INVALID_ARGUMENT);
163171

164-
switch (metadata->attributeType)
172+
switch (app::Compatibility::Internal::AttributeBaseType(metadata->attributeType))
165173
{
166174
case ZCL_BOOLEAN_ATTRIBUTE_TYPE:
167175
case ZCL_BITMAP8_ATTRIBUTE_TYPE:
168176
case ZCL_ENUM8_ATTRIBUTE_TYPE:
169177
case ZCL_INT8U_ATTRIBUTE_TYPE:
170178
VerifyOrReturnError(aVPair.valueUnsigned8.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
171-
CapAttributeID<uint8_t>(aVPair.valueUnsigned8.Value(), metadata);
179+
CapAttributeValue<uint8_t>(aVPair.valueUnsigned8.Value(), metadata);
172180
break;
173181
case ZCL_BITMAP16_ATTRIBUTE_TYPE:
174182
case ZCL_ENUM16_ATTRIBUTE_TYPE:
175183
case ZCL_INT16U_ATTRIBUTE_TYPE:
176184
VerifyOrReturnError(aVPair.valueUnsigned16.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
177-
CapAttributeID<uint16_t>(aVPair.valueUnsigned16.Value(), metadata);
185+
CapAttributeValue<uint16_t>(aVPair.valueUnsigned16.Value(), metadata);
178186
break;
179187
case ZCL_INT24U_ATTRIBUTE_TYPE:
180188
VerifyOrReturnError(aVPair.valueUnsigned32.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
181-
CapAttributeID<OddSizedInteger<3, false>>(aVPair.valueUnsigned32.Value(), metadata);
189+
CapAttributeValue<OddSizedInteger<3, false>>(aVPair.valueUnsigned32.Value(), metadata);
182190
break;
183191
case ZCL_BITMAP32_ATTRIBUTE_TYPE:
184192
case ZCL_INT32U_ATTRIBUTE_TYPE:
185193
VerifyOrReturnError(aVPair.valueUnsigned32.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
186-
CapAttributeID<uint32_t>(aVPair.valueUnsigned32.Value(), metadata);
194+
CapAttributeValue<uint32_t>(aVPair.valueUnsigned32.Value(), metadata);
195+
break;
196+
case ZCL_INT40U_ATTRIBUTE_TYPE:
197+
VerifyOrReturnError(aVPair.valueUnsigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
198+
CapAttributeValue<OddSizedInteger<5, false>>(aVPair.valueUnsigned64.Value(), metadata);
187199
break;
188200
case ZCL_INT48U_ATTRIBUTE_TYPE:
189201
VerifyOrReturnError(aVPair.valueUnsigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
190-
CapAttributeID<OddSizedInteger<6, false>>(aVPair.valueUnsigned64.Value(), metadata);
202+
CapAttributeValue<OddSizedInteger<6, false>>(aVPair.valueUnsigned64.Value(), metadata);
191203
break;
192204
case ZCL_INT56U_ATTRIBUTE_TYPE:
193205
VerifyOrReturnError(aVPair.valueUnsigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
194-
CapAttributeID<OddSizedInteger<7, false>>(aVPair.valueUnsigned64.Value(), metadata);
206+
CapAttributeValue<OddSizedInteger<7, false>>(aVPair.valueUnsigned64.Value(), metadata);
195207
break;
196208
case ZCL_BITMAP64_ATTRIBUTE_TYPE:
197209
case ZCL_INT64U_ATTRIBUTE_TYPE:
198210
VerifyOrReturnError(aVPair.valueUnsigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
199-
CapAttributeID<uint64_t>(aVPair.valueUnsigned64.Value(), metadata);
211+
CapAttributeValue<uint64_t>(aVPair.valueUnsigned64.Value(), metadata);
200212
break;
201213
case ZCL_INT8S_ATTRIBUTE_TYPE:
202214
VerifyOrReturnError(aVPair.valueSigned8.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
203-
CapAttributeID<int8_t>(aVPair.valueSigned8.Value(), metadata);
215+
CapAttributeValue<int8_t>(aVPair.valueSigned8.Value(), metadata);
204216
break;
205217
case ZCL_INT16S_ATTRIBUTE_TYPE:
206218
VerifyOrReturnError(aVPair.valueSigned16.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
207-
CapAttributeID<int16_t>(aVPair.valueSigned16.Value(), metadata);
219+
CapAttributeValue<int16_t>(aVPair.valueSigned16.Value(), metadata);
208220
break;
209221
case ZCL_INT24S_ATTRIBUTE_TYPE:
210222
VerifyOrReturnError(aVPair.valueSigned32.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
211-
CapAttributeID<OddSizedInteger<3, true>>(aVPair.valueSigned32.Value(), metadata);
223+
CapAttributeValue<OddSizedInteger<3, true>>(aVPair.valueSigned32.Value(), metadata);
212224
break;
213225
case ZCL_INT32S_ATTRIBUTE_TYPE:
214226
VerifyOrReturnError(aVPair.valueSigned32.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
215-
CapAttributeID<int32_t>(aVPair.valueSigned32.Value(), metadata);
227+
CapAttributeValue<int32_t>(aVPair.valueSigned32.Value(), metadata);
228+
break;
229+
case ZCL_INT40S_ATTRIBUTE_TYPE:
230+
VerifyOrReturnError(aVPair.valueSigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
231+
CapAttributeValue<OddSizedInteger<5, true>>(aVPair.valueSigned64.Value(), metadata);
216232
break;
217233
case ZCL_INT48S_ATTRIBUTE_TYPE:
218234
VerifyOrReturnError(aVPair.valueSigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
219-
CapAttributeID<OddSizedInteger<6, true>>(aVPair.valueSigned64.Value(), metadata);
235+
CapAttributeValue<OddSizedInteger<6, true>>(aVPair.valueSigned64.Value(), metadata);
220236
break;
221237
case ZCL_INT56S_ATTRIBUTE_TYPE:
222238
VerifyOrReturnError(aVPair.valueSigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
223-
CapAttributeID<OddSizedInteger<7, true>>(aVPair.valueSigned64.Value(), metadata);
239+
CapAttributeValue<OddSizedInteger<7, true>>(aVPair.valueSigned64.Value(), metadata);
224240
break;
225241
case ZCL_INT64S_ATTRIBUTE_TYPE:
226242
VerifyOrReturnError(aVPair.valueSigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
227-
CapAttributeID<int64_t>(aVPair.valueSigned64.Value(), metadata);
243+
CapAttributeValue<int64_t>(aVPair.valueSigned64.Value(), metadata);
228244
break;
229245
default:
230246
return CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute);

0 commit comments

Comments
 (0)