Skip to content

Commit

Permalink
Prevent any chance of division by 0 in Colorcontol. Update some C cas…
Browse files Browse the repository at this point in the history 
…ts to C++
  • Loading branch information
@jmartinez-silabs
jmartinez-silabs committed Mar 13, 2023
1 parent bc8c9ec commit 2e54b29
Showing 1 changed file with 55 additions and 21 deletions.
76 changes: 55 additions & 21 deletions src/app/clusters/color-control-server/color-control-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,9 +364,18 @@ bool ColorControlServer::computeNewColor16uValue(ColorControlServer::Color16uTra
}
else if (p->finalValue > p->initialValue)
{
newValue32u = ((uint32_t)(p->finalValue - p->initialValue));
newValue32u *= ((uint32_t)(p->stepsRemaining));
newValue32u /= ((uint32_t)(p->stepsTotal));
newValue32u = static_cast<uint32_t>(p->finalValue - p->initialValue);
newValue32u *= static_cast<uint32_t>(p->stepsRemaining);

/*
stepsTotal should always be at least 1,
still, prevent division by 0 and skips a meaningless division by 1
*/
if (p->stepsTotal > 1)
{
newValue32u /= static_cast<uint32_t>(p->stepsTotal);
}

p->currentValue = static_cast<uint16_t>(p->finalValue - static_cast<uint16_t>(newValue32u));

if (static_cast<uint16_t>(newValue32u) > p->finalValue || p->currentValue > p->highLimit)
Expand All @@ -376,9 +385,18 @@ bool ColorControlServer::computeNewColor16uValue(ColorControlServer::Color16uTra
}
else
{
newValue32u = ((uint32_t)(p->initialValue - p->finalValue));
newValue32u *= ((uint32_t)(p->stepsRemaining));
newValue32u /= ((uint32_t)(p->stepsTotal));
newValue32u = static_cast<uint32_t>(p->initialValue - p->finalValue);
newValue32u *= static_cast<uint32_t>(p->stepsRemaining);

/*
stepsTotal should always be at least 1,
still, prevent division by 0 and skips a meaningless division by 1
*/
if (p->stepsTotal > 1)
{
newValue32u /= static_cast<uint32_t>(p->stepsTotal);
}

p->currentValue = static_cast<uint16_t>(p->finalValue + static_cast<uint16_t>(newValue32u));

if (p->finalValue > UINT16_MAX - static_cast<uint16_t>(newValue32u) || p->currentValue < p->lowLimit)
Expand Down Expand Up @@ -689,7 +707,15 @@ bool ColorControlServer::computeNewHueValue(ColorControlServer::ColorHueTransiti
newHue32 = static_cast<uint32_t>(p->isEnhancedHue ? subtractEnhancedHue(p->finalEnhancedHue, p->initialEnhancedHue)
: subtractHue(p->finalHue, p->initialHue));
newHue32 *= static_cast<uint32_t>(p->stepsRemaining);
newHue32 /= static_cast<uint32_t>(p->stepsTotal);

/*
stepsTotal should always be at least 1,
still, prevent division by 0 and skips a meaningless division by 1
*/
if (p->stepsTotal > 1)
{
newHue32 /= static_cast<uint32_t>(p->stepsTotal);
}

if (p->isEnhancedHue)
{
Expand All @@ -705,7 +731,15 @@ bool ColorControlServer::computeNewHueValue(ColorControlServer::ColorHueTransiti
newHue32 = static_cast<uint32_t>(p->isEnhancedHue ? subtractEnhancedHue(p->initialEnhancedHue, p->finalEnhancedHue)
: subtractHue(p->initialHue, p->finalHue));
newHue32 *= static_cast<uint32_t>(p->stepsRemaining);
newHue32 /= static_cast<uint32_t>(p->stepsTotal);

/*
stepsTotal should always be at least 1,
still, prevent division by 0 and skips a meaningless division by 1
*/
if (p->stepsTotal > 1)
{
newHue32 /= static_cast<uint32_t>(p->stepsTotal);
}

if (p->isEnhancedHue)
{
Expand Down Expand Up @@ -1831,12 +1865,12 @@ bool ColorControlServer::moveColorCommand(app::CommandHandler * commandObj, cons
if (rateX > 0)
{
colorXTransitionState->finalValue = MAX_CIE_XY_VALUE;
unsignedRate = (uint16_t) rateX;
unsignedRate = static_cast<uint16_t>(rateX);
}
else
{
colorXTransitionState->finalValue = MIN_CIE_XY_VALUE;
unsignedRate = (uint16_t)(rateX * -1);
unsignedRate = static_cast<uint16_t>(rateX * -1);
}
transitionTimeX = computeTransitionTimeFromStateAndRate(colorXTransitionState, unsignedRate);
colorXTransitionState->stepsRemaining = transitionTimeX;
Expand All @@ -1850,12 +1884,12 @@ bool ColorControlServer::moveColorCommand(app::CommandHandler * commandObj, cons
if (rateY > 0)
{
colorYTransitionState->finalValue = MAX_CIE_XY_VALUE;
unsignedRate = (uint16_t) rateY;
unsignedRate = static_cast<uint16_t>(rateY);
}
else
{
colorYTransitionState->finalValue = MIN_CIE_XY_VALUE;
unsignedRate = (uint16_t)(rateY * -1);
unsignedRate = static_cast<uint16_t>(rateY * -1);
}
transitionTimeY = computeTransitionTimeFromStateAndRate(colorYTransitionState, unsignedRate);
colorYTransitionState->stepsRemaining = transitionTimeY;
Expand Down Expand Up @@ -2465,8 +2499,6 @@ void ColorControlServer::levelControlColorTempChangeCommand(EndpointId endpoint)

if (colorMode == COLOR_MODE_TEMPERATURE)
{
uint16_t tempCoupleMin = getTemperatureCoupleToLevelMin(endpoint);

app::DataModel::Nullable<uint8_t> currentLevel;
EmberAfStatus status = LevelControl::Attributes::CurrentLevel::Get(endpoint, currentLevel);

Expand All @@ -2475,13 +2507,14 @@ void ColorControlServer::levelControlColorTempChangeCommand(EndpointId endpoint)
currentLevel.SetNonNull((uint8_t) 0x7F);
}

uint16_t tempPhysMax = MAX_TEMPERATURE_VALUE;
Attributes::ColorTempPhysicalMaxMireds::Get(endpoint, &tempPhysMax);
uint32_t tempCoupleMin = getTemperatureCoupleToLevelMin(endpoint);
uint32_t tempPhysMax = MAX_TEMPERATURE_VALUE;
Attributes::ColorTempPhysicalMaxMireds::Get(endpoint, reinterpret_cast<uint16_t *>(&tempPhysMax));

// Scale color temp setting between the coupling min and the physical max.
// Note that mireds varies inversely with level: low level -> high mireds.
// Peg min/MAX level to MAX/min mireds, otherwise interpolate.
uint16_t newColorTemp;
uint32_t newColorTemp;
if (currentLevel.Value() <= MIN_CURRENT_LEVEL)
{
newColorTemp = tempPhysMax;
Expand All @@ -2492,13 +2525,14 @@ void ColorControlServer::levelControlColorTempChangeCommand(EndpointId endpoint)
}
else
{
uint32_t tempDelta = (((uint32_t) tempPhysMax - (uint32_t) tempCoupleMin) * currentLevel.Value()) /
(uint32_t)(MAX_CURRENT_LEVEL - MIN_CURRENT_LEVEL + 1);
newColorTemp = (uint16_t)((uint32_t) tempPhysMax - tempDelta);
uint32_t tempDelta =
((tempPhysMax - tempCoupleMin) * currentLevel.Value()) / (MAX_CURRENT_LEVEL - MIN_CURRENT_LEVEL + 1);

newColorTemp = tempPhysMax - tempDelta;
}

// Apply new color temp.
moveToColorTemp(endpoint, newColorTemp, 0);
moveToColorTemp(endpoint, static_cast<uint16_t>(newColorTemp), 0);
}
}

Expand Down

0 comments on commit 2e54b29

Please sign in to comment.