From 24c361e4e14c0fc793fdd3f5ec6d83eae68c63bb Mon Sep 17 00:00:00 2001 From: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com> Date: Mon, 18 Nov 2024 14:59:55 -0500 Subject: [PATCH 1/4] add some constraint checks to the colorcontrol cluster commands --- .../color-control-server.cpp | 60 ++++++++++++++++--- .../color-control-server.h | 2 + 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/app/clusters/color-control-server/color-control-server.cpp b/src/app/clusters/color-control-server/color-control-server.cpp index a01de7fad22dcb..3f8f48fdc0f015 100644 --- a/src/app/clusters/color-control-server/color-control-server.cpp +++ b/src/app/clusters/color-control-server/color-control-server.cpp @@ -1264,10 +1264,13 @@ EmberEventControl * ColorControlServer::configureHSVEventControl(EndpointId endp * @param saturation Target saturation * @param transitionTime Transition time in 10th of seconds * @return Status::Success When successful, - * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a saturation transition state. + * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a saturation transition state, + * Status::ConstraintError if the saturation or tansitionTime are above maximum.ß */ Status ColorControlServer::moveToSaturation(EndpointId endpoint, uint8_t saturation, uint16_t transitionTime) { + VerifyOrReturnError(saturation <= MAX_SATURATION_VALUE, Status::ConstraintError); + VerifyOrReturnError(transitionTime <= kMaxtransitionTime, Status::ConstraintError); Color16uTransitionState * colorSaturationTransitionState = getSaturationTransitionState(endpoint); VerifyOrReturnError(nullptr != colorSaturationTransitionState, Status::UnsupportedEndpoint); @@ -1307,6 +1310,7 @@ Status ColorControlServer::moveToSaturation(EndpointId endpoint, uint8_t saturat * was called by MoveHue command and rate is a uint8 value. * @return Status::Success When successful, * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a saturation transition state, + * Status::ConstraintError if the saturation or tansitionTime are above maximum. */ Status ColorControlServer::moveToHueAndSaturation(EndpointId endpoint, uint16_t hue, uint8_t saturation, uint16_t transitionTime, bool isEnhanced) @@ -1315,6 +1319,9 @@ Status ColorControlServer::moveToHueAndSaturation(EndpointId endpoint, uint16_t uint16_t halfWay = isEnhanced ? HALF_MAX_UINT16T : HALF_MAX_UINT8T; bool moveUp; + VerifyOrReturnValue(saturation <= MAX_SATURATION_VALUE, Status::ConstraintError); + VerifyOrReturnError(transitionTime <= kMaxtransitionTime, Status::ConstraintError); + uint16_t epIndex = getEndpointIndex(endpoint); Color16uTransitionState * colorSaturationTransitionState = getSaturationTransitionStateByIndex(epIndex); ColorHueTransitionState * colorHueTransitionState = getColorHueTransitionStateByIndex(epIndex); @@ -1506,6 +1513,7 @@ Status ColorControlServer::moveToHueCommand(EndpointId endpoint, uint16_t hue, D // Command Parameters constraint checks: VerifyOrReturnValue((isEnhanced || hue <= MAX_HUE_VALUE), Status::ConstraintError); + VerifyOrReturnValue(transitionTime <= kMaxtransitionTime, Status::ConstraintError); ColorHueTransitionState * colorHueTransitionState = getColorHueTransitionState(endpoint); VerifyOrReturnValue(colorHueTransitionState != nullptr, Status::UnsupportedEndpoint); @@ -1612,7 +1620,6 @@ Status ColorControlServer::moveToHueCommand(EndpointId endpoint, uint16_t hue, D * was called by MoveHue command and rate is a uint8 value. * @return Status::Success when successful, * Status::ConstraintError when the other parameters are outside their defined value range. - */ Status ColorControlServer::moveToHueAndSaturationCommand(EndpointId endpoint, uint16_t hue, uint8_t saturation, uint16_t transitionTime, BitMask optionsMask, @@ -1622,6 +1629,7 @@ Status ColorControlServer::moveToHueAndSaturationCommand(EndpointId endpoint, ui // Command Parameters constraint checks: VerifyOrReturnValue((isEnhanced || hue <= MAX_HUE_VALUE), Status::ConstraintError); VerifyOrReturnValue(saturation <= MAX_SATURATION_VALUE, Status::ConstraintError); + VerifyOrReturnValue(transitionTime <= kMaxtransitionTime, Status::ConstraintError); VerifyOrReturnValue(shouldExecuteIfOff(endpoint, optionsMask, optionsOverride), Status::Success); @@ -1656,6 +1664,16 @@ Status ColorControlServer::stepHueCommand(EndpointId endpoint, HueStepMode stepM VerifyOrReturnValue(stepMode != HueStepMode::kUnknownEnumValue, Status::InvalidCommand); VerifyOrReturnValue(stepSize != 0, Status::InvalidCommand); + // Command Parameters constraint checks: + if (isEnhanced) + { + VerifyOrReturnValue(transitionTime <= kMaxtransitionTime, Status::ConstraintError); + } + else + { + VerifyOrReturnValue(transitionTime <= UINT8_MAX, Status::ConstraintError); + } + ColorHueTransitionState * colorHueTransitionState = getColorHueTransitionState(endpoint); VerifyOrReturnValue(colorHueTransitionState != nullptr, Status::UnsupportedEndpoint); @@ -1787,7 +1805,8 @@ Status ColorControlServer::moveSaturationCommand(EndpointId endpoint, const Comm * @param commandData Struct containing the parameters of the command. * @return Status::Success when successful, * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a saturation transition state (verified in - * moveToSaturation function) Status::ConstraintError when a command parameters is outside its defined value range. + * moveToSaturation function) + * Status::ConstraintError when a command parameters is outside its defined value range. */ Status ColorControlServer::moveToSaturationCommand(EndpointId endpoint, const Commands::MoveToSaturation::DecodableType & commandData) @@ -1795,6 +1814,7 @@ Status ColorControlServer::moveToSaturationCommand(EndpointId endpoint, MATTER_TRACE_SCOPE("moveToSaturation", "ColorControl"); // Command Parameters constraint checks: VerifyOrReturnValue(commandData.saturation <= MAX_SATURATION_VALUE, Status::ConstraintError); + VerifyOrReturnValue(commandData.transitionTime <= kMaxtransitionTime, Status::ConstraintError); VerifyOrReturnValue(shouldExecuteIfOff(endpoint, commandData.optionsMask, commandData.optionsOverride), Status::Success); Status status = moveToSaturation(endpoint, commandData.saturation, commandData.transitionTime); @@ -1810,7 +1830,8 @@ Status ColorControlServer::moveToSaturationCommand(EndpointId endpoint, * @param commandData Struct containing the parameters of the command. * @return Status::Success when successful, * Status::InvalidCommand when a step size of 0 or an unknown SaturationStepMode is provided - * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a saturation transition state. + * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a saturation transition state, + * Status::ConstraintError when a command parameters is outside its defined value range. */ Status ColorControlServer::stepSaturationCommand(EndpointId endpoint, const Commands::StepSaturation::DecodableType & commandData) { @@ -1818,6 +1839,8 @@ Status ColorControlServer::stepSaturationCommand(EndpointId endpoint, const Comm // Confirm validity of the step mode and step size received VerifyOrReturnValue(commandData.stepMode != SaturationStepMode::kUnknownEnumValue, Status::InvalidCommand); VerifyOrReturnValue(commandData.stepSize != 0, Status::InvalidCommand); + // Command Parameters constraint checks: + VerifyOrReturnValue(commandData.transitionTime <= UINT8_MAX, Status::ConstraintError); Color16uTransitionState * colorSaturationTransitionState = getSaturationTransitionState(endpoint); VerifyOrReturnValue(colorSaturationTransitionState != nullptr, Status::UnsupportedEndpoint); @@ -1861,14 +1884,21 @@ Status ColorControlServer::stepSaturationCommand(EndpointId endpoint, const Comm * @param commandData Struct containing the parameters of the command. * @return Status::Success when successful, * Status::InvalidCommand when an unknown action or direction is provided - * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a hue transition state. + * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a hue transition state, + * Status::ConstraintError when a command parameters is outside its defined value range. */ Status ColorControlServer::colorLoopCommand(EndpointId endpoint, const Commands::ColorLoopSet::DecodableType & commandData) { MATTER_TRACE_SCOPE("colorLoop", "ColorControl"); + constexpr BitMask maxUpdateFlagsValue( + ColorLoopUpdateFlags::kUpdateAction, ColorLoopUpdateFlags::kUpdateDirection, ColorLoopUpdateFlags::kUpdateTime, + ColorLoopUpdateFlags::kUpdateStartHue); // add any new bitmap field to this value! + // Validate the action and direction parameters of the command VerifyOrReturnValue(commandData.action != ColorLoopActionEnum::kUnknownEnumValue, Status::InvalidCommand); VerifyOrReturnValue(commandData.direction != ColorLoopDirectionEnum::kUnknownEnumValue, Status::InvalidCommand); + // Command Parameters constraint checks: + VerifyOrReturnValue(commandData.updateFlags <= maxUpdateFlagsValue, Status::ConstraintError); uint16_t epIndex = getEndpointIndex(endpoint); ColorHueTransitionState * colorHueTransitionState = getColorHueTransitionStateByIndex(epIndex); @@ -2201,9 +2231,13 @@ Status ColorControlServer::moveToColor(EndpointId endpoint, uint16_t colorX, uin * @return Status::Success when successful, * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a Color XY transition state (verified in * moveToColor function), + * Status::ConstraintError when a command parameters is outside its defined value range. */ Status ColorControlServer::moveToColorCommand(EndpointId endpoint, const Commands::MoveToColor::DecodableType & commandData) { + // Command Parameters constraint checks: + VerifyOrReturnValue(commandData.transitionTime <= kMaxtransitionTime, Status::ConstraintError); + VerifyOrReturnValue(shouldExecuteIfOff(endpoint, commandData.optionsMask, commandData.optionsOverride), Status::Success); Status status = moveToColor(endpoint, commandData.colorX, commandData.colorY, commandData.transitionTime); @@ -2302,11 +2336,14 @@ Status ColorControlServer::moveColorCommand(EndpointId endpoint, const Commands: * @param commandData Struct containing the parameters of the command * @return Status::Success when successful, * Status::InvalidCommand when a step X and Y of 0 is provided - * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a Color XY transition state. + * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a Color XY transition state, + * Status::ConstraintError when a command parameters is outside its defined value range. */ Status ColorControlServer::stepColorCommand(EndpointId endpoint, const Commands::StepColor::DecodableType & commandData) { VerifyOrReturnValue(commandData.stepX != 0 || commandData.stepY != 0, Status::InvalidCommand); + // Command Parameters constraint checks: + VerifyOrReturnValue(commandData.transitionTime <= kMaxtransitionTime, Status::ConstraintError); uint16_t epIndex = getEndpointIndex(endpoint); Color16uTransitionState * colorXTransitionState = getXTransitionStateByIndex(epIndex); @@ -2734,11 +2771,15 @@ Status ColorControlServer::moveColorTempCommand(EndpointId endpoint, * @param commandData Struct containing the parameters of the command. * @return Status::Success when successful, * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a color XY transition state (verified in - * moveToColorTemp function). + * moveToColorTemp function), + * Status::ConstraintError when a command parameters is outside its defined value range. */ Status ColorControlServer::moveToColorTempCommand(EndpointId endpoint, const Commands::MoveToColorTemperature::DecodableType & commandData) { + // Command Parameters constraint checks: + VerifyOrReturnValue(commandData.transitionTime <= kMaxtransitionTime, Status::ConstraintError); + VerifyOrReturnValue(shouldExecuteIfOff(endpoint, commandData.optionsMask, commandData.optionsOverride), Status::Success); Status status = moveToColorTemp(endpoint, commandData.colorTemperatureMireds, commandData.transitionTime); @@ -2754,7 +2795,8 @@ Status ColorControlServer::moveToColorTempCommand(EndpointId endpoint, * @param commandData Struct containing the parameters of the command * @return Status::Success when successful, * Status::InvalidCommand when stepSize is 0 or an unknown stepMode is provided - * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a color temp transition state. + * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a color temp transition state, + * Status::ConstraintError when a command parameters is outside its defined value range. */ Status ColorControlServer::stepColorTempCommand(EndpointId endpoint, const Commands::StepColorTemperature::DecodableType & commandData) @@ -2762,6 +2804,8 @@ Status ColorControlServer::stepColorTempCommand(EndpointId endpoint, // Confirm validity of the step mode and step size received VerifyOrReturnValue(commandData.stepMode != HueStepMode::kUnknownEnumValue, Status::InvalidCommand); VerifyOrReturnValue(commandData.stepSize != 0, Status::InvalidCommand); + // Command Parameters constraint checks: + VerifyOrReturnValue(commandData.transitionTime <= kMaxtransitionTime, Status::ConstraintError); Color16uTransitionState * colorTempTransitionState = getTempTransitionState(endpoint); VerifyOrReturnValue(colorTempTransitionState != nullptr, Status::UnsupportedEndpoint); diff --git a/src/app/clusters/color-control-server/color-control-server.h b/src/app/clusters/color-control-server/color-control-server.h index 40ae30aeb2224f..6a6453f7fa0a3c 100644 --- a/src/app/clusters/color-control-server/color-control-server.h +++ b/src/app/clusters/color-control-server/color-control-server.h @@ -294,6 +294,8 @@ class ColorControlServer MATTER_DM_COLOR_CONTROL_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; static_assert(kColorControlClusterServerMaxEndpointCount <= kEmberInvalidEndpointIndex, "ColorControl endpoint count error"); + static constexpr uint16_t kMaxtransitionTime = 65534; // Max value as defined by the spec. + #ifdef MATTER_DM_PLUGIN_COLOR_CONTROL_SERVER_HSV ColorHueTransitionState colorHueTransitionStates[kColorControlClusterServerMaxEndpointCount]; Color16uTransitionState colorSatTransitionStates[kColorControlClusterServerMaxEndpointCount]; From c4a7ed8a3030ad9709e3bf70b278aa6c49f040a3 Mon Sep 17 00:00:00 2001 From: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com> Date: Mon, 18 Nov 2024 18:09:36 -0500 Subject: [PATCH 2/4] Update src/app/clusters/color-control-server/color-control-server.cpp Co-authored-by: lpbeliveau-silabs <112982107+lpbeliveau-silabs@users.noreply.github.com> --- src/app/clusters/color-control-server/color-control-server.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/clusters/color-control-server/color-control-server.cpp b/src/app/clusters/color-control-server/color-control-server.cpp index 3f8f48fdc0f015..9d1b9d9c537576 100644 --- a/src/app/clusters/color-control-server/color-control-server.cpp +++ b/src/app/clusters/color-control-server/color-control-server.cpp @@ -1265,7 +1265,7 @@ EmberEventControl * ColorControlServer::configureHSVEventControl(EndpointId endp * @param transitionTime Transition time in 10th of seconds * @return Status::Success When successful, * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a saturation transition state, - * Status::ConstraintError if the saturation or tansitionTime are above maximum.ß + * Status::ConstraintError if the saturation or tansitionTime are above maximum. */ Status ColorControlServer::moveToSaturation(EndpointId endpoint, uint8_t saturation, uint16_t transitionTime) { From 6f5bbf26b6402966e4d07d998df74237083bd887 Mon Sep 17 00:00:00 2001 From: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com> Date: Tue, 19 Nov 2024 10:00:46 -0500 Subject: [PATCH 3/4] Addrese comments. Move ConstraintCheck Before Unsupported command checks. Add contraint checks for ColorTemperatureMireds arguments --- .../color-control-server.cpp | 93 +++++++++++-------- .../color-control-server.h | 3 +- 2 files changed, 54 insertions(+), 42 deletions(-) diff --git a/src/app/clusters/color-control-server/color-control-server.cpp b/src/app/clusters/color-control-server/color-control-server.cpp index 9d1b9d9c537576..69c511f9428d27 100644 --- a/src/app/clusters/color-control-server/color-control-server.cpp +++ b/src/app/clusters/color-control-server/color-control-server.cpp @@ -1270,7 +1270,7 @@ EmberEventControl * ColorControlServer::configureHSVEventControl(EndpointId endp Status ColorControlServer::moveToSaturation(EndpointId endpoint, uint8_t saturation, uint16_t transitionTime) { VerifyOrReturnError(saturation <= MAX_SATURATION_VALUE, Status::ConstraintError); - VerifyOrReturnError(transitionTime <= kMaxtransitionTime, Status::ConstraintError); + VerifyOrReturnError(transitionTime <= kMaxTransitionTime, Status::ConstraintError); Color16uTransitionState * colorSaturationTransitionState = getSaturationTransitionState(endpoint); VerifyOrReturnError(nullptr != colorSaturationTransitionState, Status::UnsupportedEndpoint); @@ -1310,7 +1310,7 @@ Status ColorControlServer::moveToSaturation(EndpointId endpoint, uint8_t saturat * was called by MoveHue command and rate is a uint8 value. * @return Status::Success When successful, * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a saturation transition state, - * Status::ConstraintError if the saturation or tansitionTime are above maximum. + * Status::ConstraintError if the hue, saturation or tansitionTime, are above maximum. */ Status ColorControlServer::moveToHueAndSaturation(EndpointId endpoint, uint16_t hue, uint8_t saturation, uint16_t transitionTime, bool isEnhanced) @@ -1319,8 +1319,9 @@ Status ColorControlServer::moveToHueAndSaturation(EndpointId endpoint, uint16_t uint16_t halfWay = isEnhanced ? HALF_MAX_UINT16T : HALF_MAX_UINT8T; bool moveUp; - VerifyOrReturnValue(saturation <= MAX_SATURATION_VALUE, Status::ConstraintError); - VerifyOrReturnError(transitionTime <= kMaxtransitionTime, Status::ConstraintError); + VerifyOrReturnError((isEnhanced || hue <= MAX_HUE_VALUE), Status::ConstraintError); + VerifyOrReturnError(saturation <= MAX_SATURATION_VALUE, Status::ConstraintError); + VerifyOrReturnError(transitionTime <= kMaxTransitionTime, Status::ConstraintError); uint16_t epIndex = getEndpointIndex(endpoint); Color16uTransitionState * colorSaturationTransitionState = getSaturationTransitionStateByIndex(epIndex); @@ -1509,11 +1510,11 @@ Status ColorControlServer::moveToHueCommand(EndpointId endpoint, uint16_t hue, D bool isEnhanced) { MATTER_TRACE_SCOPE("moveToHue", "ColorControl"); - VerifyOrReturnValue(moveDirection != DirectionEnum::kUnknownEnumValue, Status::InvalidCommand); - // Command Parameters constraint checks: VerifyOrReturnValue((isEnhanced || hue <= MAX_HUE_VALUE), Status::ConstraintError); - VerifyOrReturnValue(transitionTime <= kMaxtransitionTime, Status::ConstraintError); + VerifyOrReturnValue(transitionTime <= kMaxTransitionTime, Status::ConstraintError); + + VerifyOrReturnValue(moveDirection != DirectionEnum::kUnknownEnumValue, Status::InvalidCommand); ColorHueTransitionState * colorHueTransitionState = getColorHueTransitionState(endpoint); VerifyOrReturnValue(colorHueTransitionState != nullptr, Status::UnsupportedEndpoint); @@ -1629,7 +1630,7 @@ Status ColorControlServer::moveToHueAndSaturationCommand(EndpointId endpoint, ui // Command Parameters constraint checks: VerifyOrReturnValue((isEnhanced || hue <= MAX_HUE_VALUE), Status::ConstraintError); VerifyOrReturnValue(saturation <= MAX_SATURATION_VALUE, Status::ConstraintError); - VerifyOrReturnValue(transitionTime <= kMaxtransitionTime, Status::ConstraintError); + VerifyOrReturnValue(transitionTime <= kMaxTransitionTime, Status::ConstraintError); VerifyOrReturnValue(shouldExecuteIfOff(endpoint, optionsMask, optionsOverride), Status::Success); @@ -1654,25 +1655,22 @@ Status ColorControlServer::moveToHueAndSaturationCommand(EndpointId endpoint, ui * @return Status::Success when successful, * Status::InvalidCommand when StepSize is 0 or an unknown HueStepMode is provided * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a hue transition state. + * Status::ConstraintError when the other parameters are outside their defined value range. */ Status ColorControlServer::stepHueCommand(EndpointId endpoint, HueStepMode stepMode, uint16_t stepSize, uint16_t transitionTime, BitMask optionsMask, BitMask optionsOverride, bool isEnhanced) { MATTER_TRACE_SCOPE("stepHue", "ColorControl"); - // Confirm validity of the step mode and step size received - VerifyOrReturnValue(stepMode != HueStepMode::kUnknownEnumValue, Status::InvalidCommand); - VerifyOrReturnValue(stepSize != 0, Status::InvalidCommand); - // Command Parameters constraint checks: + // The non-enhanced variant passed a uint8 type for transitionTime and the full range (0-255) is allowed if (isEnhanced) { - VerifyOrReturnValue(transitionTime <= kMaxtransitionTime, Status::ConstraintError); - } - else - { - VerifyOrReturnValue(transitionTime <= UINT8_MAX, Status::ConstraintError); + VerifyOrReturnValue(transitionTime <= kMaxTransitionTime, Status::ConstraintError); } + // Confirm validity of the step mode and step size received + VerifyOrReturnValue(stepMode != HueStepMode::kUnknownEnumValue, Status::InvalidCommand); + VerifyOrReturnValue(stepSize != 0, Status::InvalidCommand); ColorHueTransitionState * colorHueTransitionState = getColorHueTransitionState(endpoint); VerifyOrReturnValue(colorHueTransitionState != nullptr, Status::UnsupportedEndpoint); @@ -1806,7 +1804,7 @@ Status ColorControlServer::moveSaturationCommand(EndpointId endpoint, const Comm * @return Status::Success when successful, * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a saturation transition state (verified in * moveToSaturation function) - * Status::ConstraintError when a command parameters is outside its defined value range. + * Status::ConstraintError when a command parameter is outside its defined value range. */ Status ColorControlServer::moveToSaturationCommand(EndpointId endpoint, const Commands::MoveToSaturation::DecodableType & commandData) @@ -1814,7 +1812,7 @@ Status ColorControlServer::moveToSaturationCommand(EndpointId endpoint, MATTER_TRACE_SCOPE("moveToSaturation", "ColorControl"); // Command Parameters constraint checks: VerifyOrReturnValue(commandData.saturation <= MAX_SATURATION_VALUE, Status::ConstraintError); - VerifyOrReturnValue(commandData.transitionTime <= kMaxtransitionTime, Status::ConstraintError); + VerifyOrReturnValue(commandData.transitionTime <= kMaxTransitionTime, Status::ConstraintError); VerifyOrReturnValue(shouldExecuteIfOff(endpoint, commandData.optionsMask, commandData.optionsOverride), Status::Success); Status status = moveToSaturation(endpoint, commandData.saturation, commandData.transitionTime); @@ -1831,7 +1829,6 @@ Status ColorControlServer::moveToSaturationCommand(EndpointId endpoint, * @return Status::Success when successful, * Status::InvalidCommand when a step size of 0 or an unknown SaturationStepMode is provided * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a saturation transition state, - * Status::ConstraintError when a command parameters is outside its defined value range. */ Status ColorControlServer::stepSaturationCommand(EndpointId endpoint, const Commands::StepSaturation::DecodableType & commandData) { @@ -1839,8 +1836,6 @@ Status ColorControlServer::stepSaturationCommand(EndpointId endpoint, const Comm // Confirm validity of the step mode and step size received VerifyOrReturnValue(commandData.stepMode != SaturationStepMode::kUnknownEnumValue, Status::InvalidCommand); VerifyOrReturnValue(commandData.stepSize != 0, Status::InvalidCommand); - // Command Parameters constraint checks: - VerifyOrReturnValue(commandData.transitionTime <= UINT8_MAX, Status::ConstraintError); Color16uTransitionState * colorSaturationTransitionState = getSaturationTransitionState(endpoint); VerifyOrReturnValue(colorSaturationTransitionState != nullptr, Status::UnsupportedEndpoint); @@ -1885,20 +1880,13 @@ Status ColorControlServer::stepSaturationCommand(EndpointId endpoint, const Comm * @return Status::Success when successful, * Status::InvalidCommand when an unknown action or direction is provided * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a hue transition state, - * Status::ConstraintError when a command parameters is outside its defined value range. */ Status ColorControlServer::colorLoopCommand(EndpointId endpoint, const Commands::ColorLoopSet::DecodableType & commandData) { MATTER_TRACE_SCOPE("colorLoop", "ColorControl"); - constexpr BitMask maxUpdateFlagsValue( - ColorLoopUpdateFlags::kUpdateAction, ColorLoopUpdateFlags::kUpdateDirection, ColorLoopUpdateFlags::kUpdateTime, - ColorLoopUpdateFlags::kUpdateStartHue); // add any new bitmap field to this value! - // Validate the action and direction parameters of the command VerifyOrReturnValue(commandData.action != ColorLoopActionEnum::kUnknownEnumValue, Status::InvalidCommand); VerifyOrReturnValue(commandData.direction != ColorLoopDirectionEnum::kUnknownEnumValue, Status::InvalidCommand); - // Command Parameters constraint checks: - VerifyOrReturnValue(commandData.updateFlags <= maxUpdateFlagsValue, Status::ConstraintError); uint16_t epIndex = getEndpointIndex(endpoint); ColorHueTransitionState * colorHueTransitionState = getColorHueTransitionStateByIndex(epIndex); @@ -2176,10 +2164,18 @@ EmberEventControl * ColorControlServer::configureXYEventControl(EndpointId endpo * @param colorX target X * @param colorY target Y * @param transitionTime transition time in 10th of seconds - * @return Status::Success if successful,Status::UnsupportedEndpoint XY is not supported on the endpoint + * @return Status::Success if successful, + * @return Status::Success when successful, + * Status::UnsupportedEndpoint XY is not supported on the endpoint, + * Status::ConstraintError when a command parameter is outside its defined value range. */ Status ColorControlServer::moveToColor(EndpointId endpoint, uint16_t colorX, uint16_t colorY, uint16_t transitionTime) { + // Command Parameters constraint checks: + VerifyOrReturnValue(colorX <= MAX_CIE_XY_VALUE, Status::ConstraintError); + VerifyOrReturnValue(colorY <= MAX_CIE_XY_VALUE, Status::ConstraintError); + VerifyOrReturnValue(transitionTime <= kMaxTransitionTime, Status::ConstraintError); + uint16_t epIndex = getEndpointIndex(endpoint); Color16uTransitionState * colorXTransitionState = getXTransitionStateByIndex(epIndex); Color16uTransitionState * colorYTransitionState = getYTransitionStateByIndex(epIndex); @@ -2231,12 +2227,14 @@ Status ColorControlServer::moveToColor(EndpointId endpoint, uint16_t colorX, uin * @return Status::Success when successful, * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a Color XY transition state (verified in * moveToColor function), - * Status::ConstraintError when a command parameters is outside its defined value range. + * Status::ConstraintError when a command parameter is outside its defined value range. */ Status ColorControlServer::moveToColorCommand(EndpointId endpoint, const Commands::MoveToColor::DecodableType & commandData) { // Command Parameters constraint checks: - VerifyOrReturnValue(commandData.transitionTime <= kMaxtransitionTime, Status::ConstraintError); + VerifyOrReturnValue(commandData.colorX <= MAX_CIE_XY_VALUE, Status::ConstraintError); + VerifyOrReturnValue(commandData.colorY <= MAX_CIE_XY_VALUE, Status::ConstraintError); + VerifyOrReturnValue(commandData.transitionTime <= kMaxTransitionTime, Status::ConstraintError); VerifyOrReturnValue(shouldExecuteIfOff(endpoint, commandData.optionsMask, commandData.optionsOverride), Status::Success); @@ -2337,13 +2335,14 @@ Status ColorControlServer::moveColorCommand(EndpointId endpoint, const Commands: * @return Status::Success when successful, * Status::InvalidCommand when a step X and Y of 0 is provided * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a Color XY transition state, - * Status::ConstraintError when a command parameters is outside its defined value range. + * Status::ConstraintError when a command parameter is outside its defined value range. */ Status ColorControlServer::stepColorCommand(EndpointId endpoint, const Commands::StepColor::DecodableType & commandData) { - VerifyOrReturnValue(commandData.stepX != 0 || commandData.stepY != 0, Status::InvalidCommand); // Command Parameters constraint checks: - VerifyOrReturnValue(commandData.transitionTime <= kMaxtransitionTime, Status::ConstraintError); + VerifyOrReturnValue(commandData.transitionTime <= kMaxTransitionTime, Status::ConstraintError); + + VerifyOrReturnValue(commandData.stepX != 0 || commandData.stepY != 0, Status::InvalidCommand); uint16_t epIndex = getEndpointIndex(endpoint); Color16uTransitionState * colorXTransitionState = getXTransitionStateByIndex(epIndex); @@ -2477,8 +2476,11 @@ ColorControlServer::Color16uTransitionState * ColorControlServer::getTempTransit */ Status ColorControlServer::moveToColorTemp(EndpointId aEndpoint, uint16_t colorTemperature, uint16_t transitionTime) { - EndpointId endpoint = aEndpoint; + // Command Parameters constraint checks: + VerifyOrReturnValue(colorTemperature <= kMaxColorTemperatureMireds, Status::ConstraintError); + VerifyOrReturnValue(transitionTime <= kMaxTransitionTime, Status::ConstraintError); + EndpointId endpoint = aEndpoint; Color16uTransitionState * colorTempTransitionState = getTempTransitionState(endpoint); VerifyOrReturnError(nullptr != colorTempTransitionState, Status::UnsupportedEndpoint); @@ -2675,10 +2677,15 @@ void ColorControlServer::updateTempCommand(EndpointId endpoint) * @return Status::Success when successful, * Status::InvalidCommand when a rate of 0 for a non-stop move or an unknown HueMoveMode is provided * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a color temp transition state. + * Status::ConstraintError when a command parameter is outside its defined value range. */ Status ColorControlServer::moveColorTempCommand(EndpointId endpoint, const Commands::MoveColorTemperature::DecodableType & commandData) { + // Command Parameters constraint checks: + VerifyOrReturnValue(commandData.colorTemperatureMinimumMireds <= kMaxColorTemperatureMireds, Status::ConstraintError); + VerifyOrReturnValue(commandData.colorTemperatureMaximumMireds <= kMaxColorTemperatureMireds, Status::ConstraintError); + // check moveMode and rate before any operation is done on the transition states // rate value is ignored if the MoveMode is stop VerifyOrReturnValue(commandData.moveMode != HueMoveMode::kUnknownEnumValue, Status::InvalidCommand); @@ -2772,13 +2779,14 @@ Status ColorControlServer::moveColorTempCommand(EndpointId endpoint, * @return Status::Success when successful, * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a color XY transition state (verified in * moveToColorTemp function), - * Status::ConstraintError when a command parameters is outside its defined value range. + * Status::ConstraintError when a command parameter is outside its defined value range. */ Status ColorControlServer::moveToColorTempCommand(EndpointId endpoint, const Commands::MoveToColorTemperature::DecodableType & commandData) { // Command Parameters constraint checks: - VerifyOrReturnValue(commandData.transitionTime <= kMaxtransitionTime, Status::ConstraintError); + VerifyOrReturnValue(commandData.colorTemperatureMireds <= kMaxColorTemperatureMireds, Status::ConstraintError); + VerifyOrReturnValue(commandData.transitionTime <= kMaxTransitionTime, Status::ConstraintError); VerifyOrReturnValue(shouldExecuteIfOff(endpoint, commandData.optionsMask, commandData.optionsOverride), Status::Success); @@ -2796,16 +2804,19 @@ Status ColorControlServer::moveToColorTempCommand(EndpointId endpoint, * @return Status::Success when successful, * Status::InvalidCommand when stepSize is 0 or an unknown stepMode is provided * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a color temp transition state, - * Status::ConstraintError when a command parameters is outside its defined value range. + * Status::ConstraintError when a command parameter is outside its defined value range. */ Status ColorControlServer::stepColorTempCommand(EndpointId endpoint, const Commands::StepColorTemperature::DecodableType & commandData) { + // Command Parameters constraint checks: + VerifyOrReturnValue(commandData.transitionTime <= kMaxTransitionTime, Status::ConstraintError); + VerifyOrReturnValue(commandData.colorTemperatureMinimumMireds <= kMaxColorTemperatureMireds, Status::ConstraintError); + VerifyOrReturnValue(commandData.colorTemperatureMaximumMireds <= kMaxColorTemperatureMireds, Status::ConstraintError); + // Confirm validity of the step mode and step size received VerifyOrReturnValue(commandData.stepMode != HueStepMode::kUnknownEnumValue, Status::InvalidCommand); VerifyOrReturnValue(commandData.stepSize != 0, Status::InvalidCommand); - // Command Parameters constraint checks: - VerifyOrReturnValue(commandData.transitionTime <= kMaxtransitionTime, Status::ConstraintError); Color16uTransitionState * colorTempTransitionState = getTempTransitionState(endpoint); VerifyOrReturnValue(colorTempTransitionState != nullptr, Status::UnsupportedEndpoint); diff --git a/src/app/clusters/color-control-server/color-control-server.h b/src/app/clusters/color-control-server/color-control-server.h index 6a6453f7fa0a3c..c25313d251494e 100644 --- a/src/app/clusters/color-control-server/color-control-server.h +++ b/src/app/clusters/color-control-server/color-control-server.h @@ -294,7 +294,8 @@ class ColorControlServer MATTER_DM_COLOR_CONTROL_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; static_assert(kColorControlClusterServerMaxEndpointCount <= kEmberInvalidEndpointIndex, "ColorControl endpoint count error"); - static constexpr uint16_t kMaxtransitionTime = 65534; // Max value as defined by the spec. + static constexpr uint16_t kMaxTransitionTime = 65534; // Max value as defined by the spec. + static constexpr uint16_t kMaxColorTemperatureMireds = 65279; // Max value as defined by the spec (0xFEFF). #ifdef MATTER_DM_PLUGIN_COLOR_CONTROL_SERVER_HSV ColorHueTransitionState colorHueTransitionStates[kColorControlClusterServerMaxEndpointCount]; From 41d765eb8584e4f142a4b337f185b2ffa3d5bf10 Mon Sep 17 00:00:00 2001 From: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com> Date: Fri, 22 Nov 2024 13:08:52 -0500 Subject: [PATCH 4/4] Update src/app/clusters/color-control-server/color-control-server.cpp Co-authored-by: Boris Zbarsky --- src/app/clusters/color-control-server/color-control-server.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/clusters/color-control-server/color-control-server.cpp b/src/app/clusters/color-control-server/color-control-server.cpp index 69c511f9428d27..98379b26a70a50 100644 --- a/src/app/clusters/color-control-server/color-control-server.cpp +++ b/src/app/clusters/color-control-server/color-control-server.cpp @@ -1310,7 +1310,7 @@ Status ColorControlServer::moveToSaturation(EndpointId endpoint, uint8_t saturat * was called by MoveHue command and rate is a uint8 value. * @return Status::Success When successful, * Status::UnsupportedEndpoint when the provided endpoint doesn't correspond with a saturation transition state, - * Status::ConstraintError if the hue, saturation or tansitionTime, are above maximum. + * Status::ConstraintError if the hue, saturation or transitionTime, are above maximum. */ Status ColorControlServer::moveToHueAndSaturation(EndpointId endpoint, uint16_t hue, uint8_t saturation, uint16_t transitionTime, bool isEnhanced)