Fixes for the color control quiet reporting (#34820)

* 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 <[email protected]>
project-chip · Aug 7, 2024 · cea7fd8 · cea7fd8
1 parent 587665d
commit cea7fd8
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 14 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
@@ -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,
@@ -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;
         };
@@ -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;
 }
 
 /*
@@ -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);

diff --git a/src/app/clusters/level-control/level-control.cpp b/src/app/clusters/level-control/level-control.cpp
@@ -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
 }

diff --git a/src/python_testing/TC_CC_2_2.py b/src/python_testing/TC_CC_2_2.py
@@ -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)
@@ -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")