Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Scenes] Missing Tests Cases #32632

Merged
merged 3 commits into from
Aug 31, 2024
Merged

[Scenes] Missing Tests Cases #32632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90daf92
Implemented missing yaml tests cases and added EFS validation in Scen…
lpbeliveau-silabs Mar 18, 2024
64dbdd2
Apply suggestions from code review
lpbeliveau-silabs Aug 28, 2024
4c2702a
Cleanup up comments and useless static cast
lpbeliveau-silabs Aug 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
243 changes: 242 additions & 1 deletion src/app/clusters/scenes-server/SceneHandlerImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,249 @@
*/

#include <app/clusters/scenes-server/SceneHandlerImpl.h>
#include <app/util/ember-io-storage.h>
#include <app/util/endpoint-config-api.h>
#include <app/util/odd-sized-integers.h>

namespace chip {
namespace scenes {

namespace {

template <int ByteSize, bool IsSigned>
using OddSizedInteger = app::OddSizedInteger<ByteSize, IsSigned>;
using ConcreteAttributePath = app::ConcreteAttributePath;
using AttributeValuePairType = app::Clusters::ScenesManagement::Structs::AttributeValuePairStruct::Type;

/// ConvertDefaultValueToWorkingValue
/// @brief Helper function to convert a byte array to a value of the given type.
/// @param EmberAfDefaultAttributeValue & defaultValue
/// @return Value converted to the given working type
template <typename Type>
typename app::NumericAttributeTraits<Type>::WorkingType
ConvertDefaultValueToWorkingValue(const EmberAfDefaultAttributeValue & defaultValue)
{
if constexpr (sizeof(typename app::NumericAttributeTraits<Type>::WorkingType) <= 2)
{
return static_cast<typename app::NumericAttributeTraits<Type>::WorkingType>(defaultValue.defaultValue);
}

typename app::NumericAttributeTraits<Type>::StorageType sValue;
memcpy(&sValue, defaultValue.ptrToDefaultValue, sizeof(sValue));
return app::NumericAttributeTraits<Type>::StorageToWorking(sValue);
lpbeliveau-silabs marked this conversation as resolved.
Show resolved Hide resolved
}

/// IsExactlyOneValuePopulated
/// @brief Helper function to verify that exactly one value is populated in a given AttributeValuePairType
/// @param AttributeValuePairType & type AttributeValuePairType to verify
/// @return bool true if only one value is populated, false otherwise
bool IsExactlyOneValuePopulated(const AttributeValuePairType & type)
{
int count = 0;
if (type.valueUnsigned8.HasValue())
count++;
if (type.valueSigned8.HasValue())
count++;
if (type.valueUnsigned16.HasValue())
count++;
if (type.valueSigned16.HasValue())
count++;
if (type.valueUnsigned32.HasValue())
count++;
if (type.valueSigned32.HasValue())
count++;
if (type.valueUnsigned64.HasValue())
count++;
if (type.valueSigned64.HasValue())
count++;
return count == 1;
}

/// CapAttributeValue
/// Cap the attribute value based on the attribute's min and max if they are defined,
/// or based on the attribute's size if they are not.
/// @param[in] aVPair AttributeValuePairType
/// @param[in] metadata EmberAfAttributeMetadata
///
template <typename Type>
void CapAttributeValue(typename app::NumericAttributeTraits<Type>::WorkingType & value, const EmberAfAttributeMetadata * metadata)
{
using IntType = app::NumericAttributeTraits<Type>;
using WorkingType = typename IntType::WorkingType;

WorkingType maxValue;
WorkingType minValue;
uint16_t bitWidth = static_cast<uint16_t>(emberAfAttributeSize(metadata) * 8);

// TODO Use min/max values from Type to obtain min/max instead of relying on metadata. See:
// https://github.com/project-chip/connectedhomeip/issues/35328

// Min/Max Value caps for the OddSize integers
if (metadata->IsSignedIntegerAttribute())
{
// We use emberAfAttributeSize for cases like INT24S, INT40S, INT48S, INT56S where numeric_limits<WorkingType>::max()
lpbeliveau-silabs marked this conversation as resolved.
Show resolved Hide resolved
// wouldn't work
maxValue = static_cast<WorkingType>((1ULL << (bitWidth - 1)) - 1);
minValue = static_cast<WorkingType>(-(1ULL << (bitWidth - 1)));
}
else
{
// We use emberAfAttributeSize for cases like INT24U, INT40U, INT48U, INT56U where numeric_limits<WorkingType>::max()
// wouldn't work
if (ZCL_INT64U_ATTRIBUTE_TYPE == app::Compatibility::Internal::AttributeBaseType(metadata->attributeType))
{
maxValue = static_cast<WorkingType>(UINT64_MAX); // Bit shift of 64 is undefined so we use UINT64_MAX
}
else
{
maxValue = static_cast<WorkingType>((1ULL << bitWidth) - 1);
}
minValue = static_cast<WorkingType>(0);
}

// Ensure that the metadata's signedness matches the working type's signedness
VerifyOrDie(metadata->IsSignedIntegerAttribute() == std::is_signed<WorkingType>::value);

if (metadata->IsBoolean())
lpbeliveau-silabs marked this conversation as resolved.
Show resolved Hide resolved
{
if (metadata->IsNullable() && (value != 1 && value != 0))
{
// If the attribute is nullable, the value can be set to NULL
app::NumericAttributeTraits<WorkingType>::SetNull(value);
}
else
{
// Caping the value to 1 in case values greater than 1 are set
value = value ? 1 : 0;
}
return;
}

// Check metadata for min and max values
if (metadata->HasMinMax())
{
const EmberAfAttributeMinMaxValue * minMaxValue = metadata->defaultValue.ptrToMinMaxValue;
minValue = ConvertDefaultValueToWorkingValue<Type>(minMaxValue->minValue);
maxValue = ConvertDefaultValueToWorkingValue<Type>(minMaxValue->maxValue);
}

// If the attribute is nullable, the min and max values calculated for types will not be valid, however this does not
// 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
// -127. The code above determin minValue = -128, so an input value of -128 would not enter the condition block below, but would
// be considered NULL nonetheless.
if (metadata->IsNullable() && (minValue > value || maxValue < value))
{
// If the attribute is nullable, the value can be set to NULL
lpbeliveau-silabs marked this conversation as resolved.
Show resolved Hide resolved
app::NumericAttributeTraits<WorkingType>::SetNull(value);
return;
}

if (minValue > value)
{
value = minValue;
}
else if (maxValue < value)
{
value = maxValue;
}
}

/// @brief Validate the attribute exists for a given cluster
/// @param[in] endpoint Endpoint ID
/// @param[in] clusterID Cluster ID
/// @param[in] aVPair AttributeValuePairType, will be mutated to cap the value if it is out of range
/// @return CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute) if the attribute does not exist for a given cluster or is not scenable
/// @note This will allways fail for global list attributes. If we do want to make them scenable someday, we will need to
cecille marked this conversation as resolved.
Show resolved Hide resolved
/// use a different validation method.
// TODO: Add check for "S" quality to determine if the attribute is scenable once suported :
// https://github.com/project-chip/connectedhomeip/issues/24177
CHIP_ERROR ValidateAttributePath(EndpointId endpoint, ClusterId cluster, AttributeValuePairType & aVPair)
{
const EmberAfAttributeMetadata * metadata = emberAfLocateAttributeMetadata(endpoint, cluster, aVPair.attributeID);

if (nullptr == metadata)
{
return CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute);
}

// There should never be more than one populated value in an ExtensionFieldSet
VerifyOrReturnError(IsExactlyOneValuePopulated(aVPair), CHIP_ERROR_INVALID_ARGUMENT);

switch (app::Compatibility::Internal::AttributeBaseType(metadata->attributeType))
{
case ZCL_BOOLEAN_ATTRIBUTE_TYPE:
case ZCL_INT8U_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueUnsigned8.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<uint8_t>(aVPair.valueUnsigned8.Value(), metadata);
break;
case ZCL_INT16U_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueUnsigned16.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<uint16_t>(aVPair.valueUnsigned16.Value(), metadata);
break;
case ZCL_INT24U_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueUnsigned32.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<OddSizedInteger<3, false>>(aVPair.valueUnsigned32.Value(), metadata);
break;
case ZCL_INT32U_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueUnsigned32.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<uint32_t>(aVPair.valueUnsigned32.Value(), metadata);
break;
case ZCL_INT40U_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueUnsigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<OddSizedInteger<5, false>>(aVPair.valueUnsigned64.Value(), metadata);
break;
case ZCL_INT48U_ATTRIBUTE_TYPE:
lpbeliveau-silabs marked this conversation as resolved.
Show resolved Hide resolved
VerifyOrReturnError(aVPair.valueUnsigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<OddSizedInteger<6, false>>(aVPair.valueUnsigned64.Value(), metadata);
break;
case ZCL_INT56U_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueUnsigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<OddSizedInteger<7, false>>(aVPair.valueUnsigned64.Value(), metadata);
break;
case ZCL_INT64U_ATTRIBUTE_TYPE:
lpbeliveau-silabs marked this conversation as resolved.
Show resolved Hide resolved
VerifyOrReturnError(aVPair.valueUnsigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<uint64_t>(aVPair.valueUnsigned64.Value(), metadata);
break;
case ZCL_INT8S_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueSigned8.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<int8_t>(aVPair.valueSigned8.Value(), metadata);
break;
case ZCL_INT16S_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueSigned16.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<int16_t>(aVPair.valueSigned16.Value(), metadata);
break;
case ZCL_INT24S_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueSigned32.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<OddSizedInteger<3, true>>(aVPair.valueSigned32.Value(), metadata);
break;
case ZCL_INT32S_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueSigned32.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<int32_t>(aVPair.valueSigned32.Value(), metadata);
break;
case ZCL_INT40S_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueSigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<OddSizedInteger<5, true>>(aVPair.valueSigned64.Value(), metadata);
break;
case ZCL_INT48S_ATTRIBUTE_TYPE:
lpbeliveau-silabs marked this conversation as resolved.
Show resolved Hide resolved
VerifyOrReturnError(aVPair.valueSigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<OddSizedInteger<6, true>>(aVPair.valueSigned64.Value(), metadata);
break;
case ZCL_INT56S_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueSigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<OddSizedInteger<7, true>>(aVPair.valueSigned64.Value(), metadata);
break;
case ZCL_INT64S_ATTRIBUTE_TYPE:
VerifyOrReturnError(aVPair.valueSigned64.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
CapAttributeValue<int64_t>(aVPair.valueSigned64.Value(), metadata);
break;
default:
return CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute);
}

return CHIP_NO_ERROR;
}
} // namespace

CHIP_ERROR
DefaultSceneHandlerImpl::EncodeAttributeValueList(const List<AttributeValuePairType> & aVlist, MutableByteSpan & serializedBytes)
{
Expand Down Expand Up @@ -58,7 +297,9 @@ DefaultSceneHandlerImpl::SerializeAdd(EndpointId endpoint, const ExtensionFieldS
auto pair_iterator = extensionFieldSet.attributeValueList.begin();
while (pair_iterator.Next())
{
aVPairs[pairCount] = pair_iterator.GetValue();
AttributeValuePairType currentPair = pair_iterator.GetValue();
ReturnErrorOnFailure(ValidateAttributePath(endpoint, extensionFieldSet.clusterID, currentPair));
lpbeliveau-silabs marked this conversation as resolved.
Show resolved Hide resolved
aVPairs[pairCount] = currentPair;
pairCount++;
}
ReturnErrorOnFailure(pair_iterator.GetStatus());
Expand Down
5 changes: 5 additions & 0 deletions src/app/clusters/scenes-server/scenes-server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ CHIP_ERROR AddResponseOnError(CommandHandlerInterface::HandlerContext & ctx, Res
{
resp.status = to_underlying(Protocols::InteractionModel::Status::ResourceExhausted);
}
else if (CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute) == err)
{
// TODO: Confirm if we need to add UnsupportedAttribute status as a return for Scene Commands
resp.status = to_underlying(Protocols::InteractionModel::Status::InvalidCommand);
}
else
{
resp.status = to_underlying(StatusIB(err).mStatus);
Expand Down
Loading
Loading