Skip to content

Commit 3b83878

Browse files
jmartinez-silabsrestyled-commitsbzbarsky-apple
authored
Update Quiet reporting conditions for LVL and CC remaining time attribute (project-chip#35224)
* Update Quiet reporting conditions for LVL and CC remaining time attribute * Restyled by clang-format * Fix/cleaup of startUpColorTempCommand * Add missing SetQuietReportRemainingTime for moveToColorTemp command. Add remainingTime Report condition on command invoke. Fix clang-tidy * Update src/app/clusters/color-control-server/color-control-server.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
1 parent ee40a18 commit 3b83878

File tree

3 files changed

+130
-113
lines changed

3 files changed

+130
-113
lines changed

‎src/app/clusters/color-control-server/color-control-server.cpp

+82-71
Original file line numberDiff line numberDiff line change
@@ -1040,7 +1040,7 @@ void ColorControlServer::startColorLoop(EndpointId endpoint, uint8_t startFromSt
10401040
colorHueTransitionState->transitionTime = MAX_INT16U_VALUE;
10411041
colorHueTransitionState->endpoint = endpoint;
10421042

1043-
SetQuietReportRemainingTime(endpoint, MAX_INT16U_VALUE);
1043+
SetQuietReportRemainingTime(endpoint, MAX_INT16U_VALUE, true /* isNewTransition */);
10441044

10451045
scheduleTimerCallbackMs(configureHSVEventControl(endpoint), TRANSITION_UPDATE_TIME_MS.count());
10461046
}
@@ -1091,7 +1091,10 @@ void ColorControlServer::SetHSVRemainingTime(chip::EndpointId endpoint)
10911091
// When the hue transition is loop, RemainingTime stays at MAX_INT16
10921092
if (hueTransitionState->repeat == false)
10931093
{
1094-
SetQuietReportRemainingTime(endpoint, max(hueTransitionState->timeRemaining, saturationTransitionState->timeRemaining));
1094+
bool hsvTransitionStart = (hueTransitionState->stepsRemaining == hueTransitionState->stepsTotal) ||
1095+
(saturationTransitionState->stepsRemaining == saturationTransitionState->stepsTotal);
1096+
SetQuietReportRemainingTime(endpoint, max(hueTransitionState->timeRemaining, saturationTransitionState->timeRemaining),
1097+
hsvTransitionStart);
10951098
}
10961099
}
10971100

@@ -1484,7 +1487,7 @@ bool ColorControlServer::moveHueCommand(app::CommandHandler * commandObj, const
14841487
colorHueTransitionState->repeat = true;
14851488

14861489
// hue movement can last forever. Indicate this with a remaining time of maxint
1487-
SetQuietReportRemainingTime(endpoint, MAX_INT16U_VALUE);
1490+
SetQuietReportRemainingTime(endpoint, MAX_INT16U_VALUE, true /* isNewTransition */);
14881491

14891492
// kick off the state machine:
14901493
scheduleTimerCallbackMs(configureHSVEventControl(endpoint), TRANSITION_UPDATE_TIME_MS.count());
@@ -2052,7 +2055,7 @@ bool ColorControlServer::colorLoopCommand(app::CommandHandler * commandObj, cons
20522055
uint16_t storedEnhancedHue = 0;
20532056
Attributes::ColorLoopStoredEnhancedHue::Get(endpoint, &storedEnhancedHue);
20542057
MarkAttributeDirty markDirty =
2055-
SetQuietReportAttribute(quietEnhancedHue[epIndex], storedEnhancedHue, true /*isStartOrEndOfTransition*/, 0);
2058+
SetQuietReportAttribute(quietEnhancedHue[epIndex], storedEnhancedHue, true /*isEndOfTransition*/, 0);
20562059
Attributes::EnhancedCurrentHue::Set(endpoint, quietEnhancedHue[epIndex].value().Value(), markDirty);
20572060
}
20582061
else
@@ -2094,10 +2097,6 @@ void ColorControlServer::updateHueSatCommand(EndpointId endpoint)
20942097
uint16_t previousSaturation = colorSaturationTransitionState->currentValue;
20952098
uint16_t previousEnhancedhue = colorHueTransitionState->currentEnhancedHue;
20962099

2097-
bool isHueTansitionStart = (colorHueTransitionState->stepsRemaining == colorHueTransitionState->stepsTotal);
2098-
bool isSaturationTransitionStart =
2099-
(colorSaturationTransitionState->stepsRemaining == colorSaturationTransitionState->stepsTotal);
2100-
21012100
bool isHueTansitionDone = computeNewHueValue(colorHueTransitionState);
21022101
bool isSaturationTransitionDone = computeNewColor16uValue(colorSaturationTransitionState);
21032102

@@ -2117,7 +2116,7 @@ void ColorControlServer::updateHueSatCommand(EndpointId endpoint)
21172116
if (colorHueTransitionState->isEnhancedHue)
21182117
{
21192118
markDirty = SetQuietReportAttribute(quietEnhancedHue[epIndex], colorHueTransitionState->currentEnhancedHue,
2120-
(isHueTansitionStart || isHueTansitionDone), colorHueTransitionState->transitionTime);
2119+
isHueTansitionDone, colorHueTransitionState->transitionTime);
21212120
Attributes::EnhancedCurrentHue::Set(endpoint, quietEnhancedHue[epIndex].value().Value(), markDirty);
21222121
currentHue = static_cast<uint8_t>(colorHueTransitionState->currentEnhancedHue >> 8);
21232122

@@ -2135,8 +2134,7 @@ void ColorControlServer::updateHueSatCommand(EndpointId endpoint)
21352134
}
21362135
}
21372136

2138-
markDirty = SetQuietReportAttribute(quietHue[epIndex], currentHue, (isHueTansitionStart || isHueTansitionDone),
2139-
colorHueTransitionState->transitionTime);
2137+
markDirty = SetQuietReportAttribute(quietHue[epIndex], currentHue, isHueTansitionDone, colorHueTransitionState->transitionTime);
21402138
Attributes::CurrentHue::Set(endpoint, quietHue[epIndex].value().Value(), markDirty);
21412139

21422140
if (previousSaturation != colorSaturationTransitionState->currentValue)
@@ -2145,8 +2143,7 @@ void ColorControlServer::updateHueSatCommand(EndpointId endpoint)
21452143
}
21462144

21472145
markDirty = SetQuietReportAttribute(quietSaturation[epIndex], colorSaturationTransitionState->currentValue,
2148-
(isSaturationTransitionStart || isSaturationTransitionDone),
2149-
colorSaturationTransitionState->transitionTime);
2146+
isSaturationTransitionDone, colorSaturationTransitionState->transitionTime);
21502147
Attributes::CurrentSaturation::Set(endpoint, quietSaturation[epIndex].value().Value(), markDirty);
21512148

21522149
computePwmFromHsv(endpoint);
@@ -2298,7 +2295,7 @@ Status ColorControlServer::moveToColor(uint16_t colorX, uint16_t colorY, uint16_
22982295
colorYTransitionState->lowLimit = MIN_CIE_XY_VALUE;
22992296
colorYTransitionState->highLimit = MAX_CIE_XY_VALUE;
23002297

2301-
SetQuietReportRemainingTime(endpoint, transitionTime);
2298+
SetQuietReportRemainingTime(endpoint, transitionTime, true /* isNewTransition */);
23022299

23032300
// kick off the state machine:
23042301
scheduleTimerCallbackMs(configureXYEventControl(endpoint), transitionTime ? TRANSITION_UPDATE_TIME_MS.count() : 0);
@@ -2405,7 +2402,7 @@ bool ColorControlServer::moveColorCommand(app::CommandHandler * commandObj, cons
24052402
colorYTransitionState->lowLimit = MIN_CIE_XY_VALUE;
24062403
colorYTransitionState->highLimit = MAX_CIE_XY_VALUE;
24072404

2408-
SetQuietReportRemainingTime(endpoint, max(transitionTimeX, transitionTimeY));
2405+
SetQuietReportRemainingTime(endpoint, max(transitionTimeX, transitionTimeY), true /* isNewTransition */);
24092406

24102407
// kick off the state machine:
24112408
scheduleTimerCallbackMs(configureXYEventControl(endpoint), TRANSITION_UPDATE_TIME_MS.count());
@@ -2485,7 +2482,7 @@ bool ColorControlServer::stepColorCommand(app::CommandHandler * commandObj, cons
24852482
colorYTransitionState->lowLimit = MIN_CIE_XY_VALUE;
24862483
colorYTransitionState->highLimit = MAX_CIE_XY_VALUE;
24872484

2488-
SetQuietReportRemainingTime(endpoint, transitionTime);
2485+
SetQuietReportRemainingTime(endpoint, transitionTime, true /* isNewTransition */);
24892486

24902487
// kick off the state machine:
24912488
scheduleTimerCallbackMs(configureXYEventControl(endpoint), transitionTime ? TRANSITION_UPDATE_TIME_MS.count() : 0);
@@ -2521,15 +2518,10 @@ void ColorControlServer::updateXYCommand(EndpointId endpoint)
25212518
scheduleTimerCallbackMs(configureXYEventControl(endpoint), TRANSITION_UPDATE_TIME_MS.count());
25222519
}
25232520

2524-
bool isXTransitionStart = (colorXTransitionState->stepsRemaining == colorXTransitionState->stepsTotal);
2525-
bool isYTransitionStart = (colorYTransitionState->stepsRemaining == colorYTransitionState->stepsTotal);
2526-
2527-
MarkAttributeDirty markXDirty =
2528-
SetQuietReportAttribute(quietColorX[epIndex], colorXTransitionState->currentValue,
2529-
(isXTransitionStart || isXTransitionDone), colorXTransitionState->transitionTime);
2530-
MarkAttributeDirty markYDirty =
2531-
SetQuietReportAttribute(quietColorY[epIndex], colorYTransitionState->currentValue,
2532-
(isYTransitionStart || isYTransitionDone), colorYTransitionState->transitionTime);
2521+
MarkAttributeDirty markXDirty = SetQuietReportAttribute(quietColorX[epIndex], colorXTransitionState->currentValue,
2522+
isXTransitionDone, colorXTransitionState->transitionTime);
2523+
MarkAttributeDirty markYDirty = SetQuietReportAttribute(quietColorY[epIndex], colorYTransitionState->currentValue,
2524+
isYTransitionDone, colorYTransitionState->transitionTime);
25332525

25342526
Attributes::CurrentX::Set(endpoint, quietColorX[epIndex].value().Value(), markXDirty);
25352527
Attributes::CurrentY::Set(endpoint, quietColorY[epIndex].value().Value(), markYDirty);
@@ -2623,6 +2615,8 @@ Status ColorControlServer::moveToColorTemp(EndpointId aEndpoint, uint16_t colorT
26232615
colorTempTransitionState->lowLimit = temperatureMin;
26242616
colorTempTransitionState->highLimit = temperatureMax;
26252617

2618+
SetQuietReportRemainingTime(endpoint, transitionTime, true /* isNewTransition */);
2619+
26262620
// kick off the state machine
26272621
scheduleTimerCallbackMs(configureTempEventControl(endpoint), transitionTime ? TRANSITION_UPDATE_TIME_MS.count() : 0);
26282622
return Status::Success;
@@ -2687,36 +2681,32 @@ void ColorControlServer::startUpColorTempCommand(EndpointId endpoint)
26872681

26882682
if (status == Status::Success && !startUpColorTemp.IsNull())
26892683
{
2690-
uint16_t updatedColorTemp = MAX_TEMPERATURE_VALUE;
2691-
status = Attributes::ColorTemperatureMireds::Get(endpoint, &updatedColorTemp);
2684+
uint16_t tempPhysicalMin = MIN_TEMPERATURE_VALUE;
2685+
Attributes::ColorTempPhysicalMinMireds::Get(endpoint, &tempPhysicalMin);
2686+
// Avoid potential divide-by-zero in future Kelvin conversions.
2687+
tempPhysicalMin = std::max(static_cast<uint16_t>(1u), tempPhysicalMin);
26922688

2693-
if (status == Status::Success)
2694-
{
2695-
uint16_t tempPhysicalMin = MIN_TEMPERATURE_VALUE;
2696-
Attributes::ColorTempPhysicalMinMireds::Get(endpoint, &tempPhysicalMin);
2697-
// Avoid potential divide-by-zero in future Kelvin conversions.
2698-
tempPhysicalMin = std::max(static_cast<uint16_t>(1u), tempPhysicalMin);
2689+
uint16_t tempPhysicalMax = MAX_TEMPERATURE_VALUE;
2690+
Attributes::ColorTempPhysicalMaxMireds::Get(endpoint, &tempPhysicalMax);
26992691

2700-
uint16_t tempPhysicalMax = MAX_TEMPERATURE_VALUE;
2701-
Attributes::ColorTempPhysicalMaxMireds::Get(endpoint, &tempPhysicalMax);
2692+
if (tempPhysicalMin <= startUpColorTemp.Value() && startUpColorTemp.Value() <= tempPhysicalMax)
2693+
{
2694+
// Apply valid startup color temp value that is within physical limits of device.
2695+
// Otherwise, the startup value is outside the device's supported range, and the
2696+
// existing setting of ColorTemp attribute will be left unchanged (i.e., treated as
2697+
// if startup color temp was set to null).
2698+
uint16_t epIndex = getEndpointIndex(endpoint);
2699+
MarkAttributeDirty markDirty = SetQuietReportAttribute(quietTemperatureMireds[epIndex], startUpColorTemp.Value(),
2700+
false /* isEndOfTransition */, 0);
2701+
status = Attributes::ColorTemperatureMireds::Set(endpoint, quietTemperatureMireds[epIndex].value().Value(), markDirty);
27022702

2703-
if (tempPhysicalMin <= startUpColorTemp.Value() && startUpColorTemp.Value() <= tempPhysicalMax)
2703+
if (status == Status::Success)
27042704
{
2705-
// Apply valid startup color temp value that is within physical limits of device.
2706-
// Otherwise, the startup value is outside the device's supported range, and the
2707-
// existing setting of ColorTemp attribute will be left unchanged (i.e., treated as
2708-
// if startup color temp was set to null).
2709-
updatedColorTemp = startUpColorTemp.Value();
2710-
status = Attributes::ColorTemperatureMireds::Set(endpoint, updatedColorTemp);
2711-
2712-
if (status == Status::Success)
2713-
{
2714-
// Set ColorMode attributes to reflect ColorTemperature.
2715-
auto updateColorMode = ColorModeEnum::kColorTemperatureMireds;
2716-
Attributes::ColorMode::Set(endpoint, updateColorMode);
2705+
// Set ColorMode attributes to reflect ColorTemperature.
2706+
auto updateColorMode = ColorModeEnum::kColorTemperatureMireds;
2707+
Attributes::ColorMode::Set(endpoint, updateColorMode);
27172708

2718-
Attributes::EnhancedColorMode::Set(endpoint, static_cast<EnhancedColorModeEnum>(updateColorMode));
2719-
}
2709+
Attributes::EnhancedColorMode::Set(endpoint, static_cast<EnhancedColorModeEnum>(updateColorMode));
27202710
}
27212711
}
27222712
}
@@ -2729,7 +2719,8 @@ void ColorControlServer::startUpColorTempCommand(EndpointId endpoint)
27292719
*/
27302720
void ColorControlServer::updateTempCommand(EndpointId endpoint)
27312721
{
2732-
Color16uTransitionState * colorTempTransitionState = getTempTransitionState(endpoint);
2722+
uint16_t epIndex = getEndpointIndex(endpoint);
2723+
Color16uTransitionState * colorTempTransitionState = getTempTransitionStateByIndex(epIndex);
27332724
bool isColorTempTransitionDone;
27342725

27352726
isColorTempTransitionDone = computeNewColor16uValue(colorTempTransitionState);
@@ -2763,7 +2754,9 @@ void ColorControlServer::updateTempCommand(EndpointId endpoint)
27632754
scheduleTimerCallbackMs(configureTempEventControl(endpoint), TRANSITION_UPDATE_TIME_MS.count());
27642755
}
27652756

2766-
Attributes::ColorTemperatureMireds::Set(endpoint, colorTempTransitionState->currentValue);
2757+
MarkAttributeDirty markDirty = SetQuietReportAttribute(quietTemperatureMireds[epIndex], colorTempTransitionState->currentValue,
2758+
isColorTempTransitionDone, colorTempTransitionState->timeRemaining);
2759+
Attributes::ColorTemperatureMireds::Set(endpoint, quietTemperatureMireds[epIndex].value().Value(), markDirty);
27672760

27682761
ChipLogProgress(Zcl, "Color Temperature %d", colorTempTransitionState->currentValue);
27692762

@@ -2882,7 +2875,7 @@ bool ColorControlServer::moveColorTempCommand(app::CommandHandler * commandObj,
28822875
colorTempTransitionState->lowLimit = colorTemperatureMinimum;
28832876
colorTempTransitionState->highLimit = colorTemperatureMaximum;
28842877

2885-
SetQuietReportRemainingTime(endpoint, transitionTime);
2878+
SetQuietReportRemainingTime(endpoint, transitionTime, true /* isNewTransition */);
28862879

28872880
// kick off the state machine:
28882881
scheduleTimerCallbackMs(configureTempEventControl(endpoint), TRANSITION_UPDATE_TIME_MS.count());
@@ -3005,7 +2998,7 @@ bool ColorControlServer::stepColorTempCommand(app::CommandHandler * commandObj,
30052998
colorTempTransitionState->lowLimit = colorTemperatureMinimum;
30062999
colorTempTransitionState->highLimit = colorTemperatureMaximum;
30073000

3008-
SetQuietReportRemainingTime(endpoint, transitionTime);
3001+
SetQuietReportRemainingTime(endpoint, transitionTime, true /* isNewTransition */);
30093002

30103003
// kick off the state machine:
30113004
scheduleTimerCallbackMs(configureTempEventControl(endpoint), transitionTime ? TRANSITION_UPDATE_TIME_MS.count() : 0);
@@ -3103,7 +3096,6 @@ void ColorControlServer::levelControlColorTempChangeCommand(EndpointId endpoint)
31033096
* Utility function used to update a color control attribute which has the quiet reporting quality.
31043097
* matching the following report conditions:
31053098
* - At most once per second, or
3106-
* - At the start of the movement/transition, or
31073099
* - At the end of the movement/transition, or
31083100
* - When it changes from null to any other value and vice versa. (Implicit to the QuieterReportingAttribute class)
31093101
*
@@ -3114,20 +3106,20 @@ void ColorControlServer::levelControlColorTempChangeCommand(EndpointId endpoint)
31143106
*
31153107
* @param quietReporter: The QuieterReportingAttribute<TYPE> object for the attribute to update.
31163108
* @param newValue: Value to update the attribute with
3117-
* @param isStartOrEndOfTransition: Boolean that indicatse whether the update is occurring at the start or end of a level transition
3109+
* @param isEndOfTransition: Boolean that indicates whether the update is occurring at the end of a color transition
31183110
* @return MarkAttributeDirty::kYes when the attribute must be marked dirty and be reported. MarkAttributeDirty::kNo when
31193111
* no report is needed.
31203112
*/
31213113
template <typename Q, typename V>
31223114
MarkAttributeDirty ColorControlServer::SetQuietReportAttribute(QuieterReportingAttribute<Q> & quietReporter, V newValue,
3123-
bool isStartOrEndOfTransition, uint16_t transitionTime)
3115+
bool isEndOfTransition, uint16_t transitionTime)
31243116
{
31253117
AttributeDirtyState dirtyState;
31263118
auto now = System::SystemClock().GetMonotonicTimestamp();
31273119

3128-
if (isStartOrEndOfTransition)
3120+
if (isEndOfTransition)
31293121
{
3130-
// At the start or end of the movement/transition we must report if the value changed
3122+
// At the end of the movement/transition we must report if the value changed
31313123
auto predicate = [](const typename QuieterReportingAttribute<Q>::SufficientChangePredicateCandidate &) -> bool {
31323124
return true;
31333125
};
@@ -3157,23 +3149,42 @@ MarkAttributeDirty ColorControlServer::SetQuietReportAttribute(QuieterReportingA
31573149
* @brief
31583150
* Function used to set the remaining time based on quiet reporting conditions.
31593151
* It will update the attribute storage and report the attribute if it is determined dirty.
3160-
* The condition on which the attribute must be reported are defined by the set QuieterReportingPolicyFlags
3161-
* of the quietRemainingTime object and the implicit conditions of the QuieterReportingAttribute class
3152+
* The conditions on which the attribute must be reported are:
3153+
* - When it changes from 0 to any value higher than 10, or
3154+
* - When it changes, with a delta larger than 10, caused by the invoke of a command, or
3155+
* - When it changes to 0.
31623156
*
31633157
* @param endpoint: Endpoint of the RemainingTime attribute to set
31643158
* @param newRemainingTime: Value to update the RemainingTime attribute with
31653159
* @return Success in setting the attribute value or the IM error code for the failure.
31663160
*/
3167-
Status ColorControlServer::SetQuietReportRemainingTime(EndpointId endpoint, uint16_t newRemainingTime)
3168-
{
3169-
uint16_t epIndex = getEndpointIndex(endpoint);
3170-
auto markDirty = MarkAttributeDirty::kNo;
3171-
auto now = System::SystemClock().GetMonotonicTimestamp();
3172-
// Establish the quiet report condition for the RemainingTime Attribute
3173-
// The quiet report is by the previously set policies :
3174-
// - kMarkDirtyOnChangeToFromZero : When the value changes from 0 to any other value and vice versa, or
3175-
// - kMarkDirtyOnIncrement : When the value increases.
3176-
if (quietRemainingTime[epIndex].SetValue(newRemainingTime, now) == AttributeDirtyState::kMustReport)
3161+
Status ColorControlServer::SetQuietReportRemainingTime(EndpointId endpoint, uint16_t newRemainingTime, bool isNewTransition)
3162+
{
3163+
uint16_t epIndex = getEndpointIndex(endpoint);
3164+
uint16_t lastRemainingTime = quietRemainingTime[epIndex].value().ValueOr(0);
3165+
auto markDirty = MarkAttributeDirty::kNo;
3166+
auto now = System::SystemClock().GetMonotonicTimestamp();
3167+
3168+
auto predicate =
3169+
[isNewTransition, lastRemainingTime](
3170+
const typename QuieterReportingAttribute<uint16_t>::SufficientChangePredicateCandidate & candidate) -> bool {
3171+
constexpr uint16_t reportDelta = 10;
3172+
bool isDirty = false;
3173+
if (candidate.newValue.Value() == 0 || (candidate.lastDirtyValue.Value() == 0 && candidate.newValue.Value() > reportDelta))
3174+
{
3175+
isDirty = true;
3176+
}
3177+
else if (isNewTransition &&
3178+
(candidate.newValue.Value() > static_cast<uint32_t>(lastRemainingTime + reportDelta) ||
3179+
static_cast<uint32_t>(candidate.newValue.Value() + reportDelta) < lastRemainingTime ||
3180+
candidate.newValue.Value() > static_cast<uint32_t>(candidate.lastDirtyValue.Value() + reportDelta)))
3181+
{
3182+
isDirty = true;
3183+
}
3184+
return isDirty;
3185+
};
3186+
3187+
if (quietRemainingTime[epIndex].SetValue(newRemainingTime, now, predicate) == AttributeDirtyState::kMustReport)
31773188
{
31783189
markDirty = MarkAttributeDirty::kYes;
31793190
}

0 commit comments

Comments
 (0)