Skip to content

Commit

Permalink
Fixes for the color control quiet reporting (#34820)
Browse files Browse the repository at this point in the history
* MarkAttributeDirty::kYes instead of kIfChanged for QuietReporting to insure the value is reported. Fix Start Or End of transition Quiet report to only report if the attribute value changed since the last quiet report

* apply same tweak for start/end of transition to the lvl control.

* re-enable the disabled tc_cc_2_2 steps

* Restyled by autopep8

* address comments

---------

Co-authored-by: Restyled.io <commits@restyled.io>
  • Loading branch information
jmartinez-silabs and restyled-commits authored Aug 7, 2024
1 parent 587665d commit cea7fd8
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 14 deletions.
12 changes: 6 additions & 6 deletions src/app/clusters/color-control-server/color-control-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3113,15 +3113,15 @@ void ColorControlServer::levelControlColorTempChangeCommand(EndpointId endpoint)
* - When it changes from null to any other value and vice versa. (Implicit to the QuieterReportingAttribute class)
*
* The QuietReportAttribute class is updated with the new value and when the report conditions are met,
* this function will return MarkAttributeDirty::kIfChanged.
* this function will return MarkAttributeDirty::kYes.
* It is expected that the user will use this return value to trigger a reporting mechanism for the attribute with the new value
* (Which was updated in the quietReporter)
*
* @param quietReporter: The QuieterReportingAttribute<TYPE> object for the attribute to update.
* @param newValue: Value to update the attribute with
* @param isStartOrEndOfTransition: Boolean that indicatse whether the update is occurring at the start or end of a level transition
* @return MarkAttributeDirty::kIfChanged when the attribute must be maredk dirty and be reported. MarkAttributeDirty::kNo when it
* when it no report is needed.
* @return MarkAttributeDirty::kYes when the attribute must be marked dirty and be reported. MarkAttributeDirty::kNo when
* no report is needed.
*/
template <typename Q, typename V>
MarkAttributeDirty ColorControlServer::SetQuietReportAttribute(QuieterReportingAttribute<Q> & quietReporter, V newValue,
Expand All @@ -3132,7 +3132,7 @@ MarkAttributeDirty ColorControlServer::SetQuietReportAttribute(QuieterReportingA

if (isStartOrEndOfTransition)
{
// At the start or end of the movement/transition we must report
// At the start or end of the movement/transition we must report if the value changed
auto predicate = [](const typename QuieterReportingAttribute<Q>::SufficientChangePredicateCandidate &) -> bool {
return true;
};
Expand All @@ -3155,7 +3155,7 @@ MarkAttributeDirty ColorControlServer::SetQuietReportAttribute(QuieterReportingA
dirtyState = quietReporter.SetValue(newValue, now, predicate);
}

return (dirtyState == AttributeDirtyState::kMustReport) ? MarkAttributeDirty::kIfChanged : MarkAttributeDirty::kNo;
return (dirtyState == AttributeDirtyState::kMustReport) ? MarkAttributeDirty::kYes : MarkAttributeDirty::kNo;
}

/*
Expand All @@ -3180,7 +3180,7 @@ Status ColorControlServer::SetQuietReportRemainingTime(EndpointId endpoint, uint
// - kMarkDirtyOnIncrement : When the value increases.
if (quietRemainingTime[epIndex].SetValue(newRemainingTime, now) == AttributeDirtyState::kMustReport)
{
markDirty = MarkAttributeDirty::kIfChanged;
markDirty = MarkAttributeDirty::kYes;
}

return Attributes::RemainingTime::Set(endpoint, quietRemainingTime[epIndex].value().Value(), markDirty);
Expand Down
2 changes: 1 addition & 1 deletion src/app/clusters/level-control/level-control.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ static void writeRemainingTime(EndpointId endpoint, uint16_t remainingTimeMs)
markDirty = MarkAttributeDirty::kYes;
}

Attributes::RemainingTime::Set(endpoint, state->quietRemainingTime.value().ValueOr(0), markDirty);
Attributes::RemainingTime::Set(endpoint, state->quietRemainingTime.value().Value(), markDirty);
}
#endif // IGNORE_LEVEL_CONTROL_CLUSTER_LEVEL_CONTROL_REMAINING_TIME
}
Expand Down
12 changes: 5 additions & 7 deletions src/python_testing/TC_CC_2_2.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,7 @@ def accumulate_reports():

def check_report_counts(attr: ClusterObjects.ClusterAttributeDescriptor):
count = sub_handler.attribute_report_counts[attr]
# TODO: should be 12 - see issue #34646
# asserts.assert_less_equal(count, 12, "More than 12 reports received")
asserts.assert_less_equal(count, 12, "More than 12 reports received")
asserts.assert_less_equal(count, gather_time, f"More than {gather_time} reports received")

self.step(9)
Expand Down Expand Up @@ -270,17 +269,16 @@ def check_report_counts(attr: ClusterObjects.ClusterAttributeDescriptor):
time.sleep(20)

self.step(34)
# TODO: Re-enable checks 34, 36 when #34643 is addressed
logging.info(f'received reports: {sub_handler.attribute_reports[cc.Attributes.RemainingTime]}')
# count = sub_handler.attribute_report_counts[cc.Attributes.RemainingTime]
# asserts.assert_equal(count, 3, "Unexpected number of reports received")
count = sub_handler.attribute_report_counts[cc.Attributes.RemainingTime]
asserts.assert_equal(count, 3, "Unexpected number of reports received")

self.step(35)
asserts.assert_equal(sub_handler.attribute_reports[cc.Attributes.RemainingTime][0].value, 100, "Unexpected first report")

self.step(36)
# asserts.assert_almost_equal(
# sub_handler.attribute_reports[cc.Attributes.RemainingTime][1].value, 0, delta=10, msg="Unexpected second report")
asserts.assert_almost_equal(
sub_handler.attribute_reports[cc.Attributes.RemainingTime][1].value, 150, delta=10, msg="Unexpected second report")

self.step(37)
asserts.assert_equal(sub_handler.attribute_reports[cc.Attributes.RemainingTime][-1].value, 0, "Unexpected last report")
Expand Down

0 comments on commit cea7fd8

Please sign in to comment.