From 64423be246ae906e126938aebbaa3a3ee554ed32 Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Thu, 25 Jul 2024 16:34:40 -0400
Subject: [PATCH] Add a way to force reporting when setting attr value in ember
 store.

When using a QuieterReportingAttribute to track whether reporting is needed, we
can end up in a situation where we set a value that we have not reported yet,
and then an edge happens that requires reporting to be triggered, without the
relevant value changing.  In that situation the QuieterReportingAttribute
remembers the last-reported value (not just the last-set one), and will return
AttributeDirtyState::kMustReport.

What was missing was a way to tell the ember data store "go ahead and report
this, even though we are not in fact changing the value".  We want that, in this
case, since the ember data store just knows the last-set value, not the
last-reported one.
---
 src/app/clusters/level-control/level-control.cpp | 10 +++++-----
 src/app/util/af-types.h                          |  5 +++++
 src/app/util/attribute-table.cpp                 |  7 ++++++-
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/app/clusters/level-control/level-control.cpp b/src/app/clusters/level-control/level-control.cpp
index 814436c39e179a..3963c6d8ac5829 100644
--- a/src/app/clusters/level-control/level-control.cpp
+++ b/src/app/clusters/level-control/level-control.cpp
@@ -372,7 +372,7 @@ static void reallyUpdateCoupledColorTemp(EndpointId endpoint)
 /*
  * @brief
  * This function is used to update the current level attribute
- * while respecting it's defined quiet reporting quality:
+ * while respecting its defined quiet reporting quality:
  * The attribute will be reported:
  * - At most once per second, or
  * - At the start of the movement/transition, or
@@ -389,8 +389,7 @@ static Status SetCurrentLevelQuietReport(EndpointId endpoint, EmberAfLevelContro
                                          DataModel::Nullable<uint8_t> newValue, bool isStartOrEndOfTransition)
 {
     AttributeDirtyState dirtyState;
-    MarkAttributeDirty markDirty = MarkAttributeDirty::kNo;
-    auto now                     = System::SystemClock().GetMonotonicTimestamp();
+    auto now = System::SystemClock().GetMonotonicTimestamp();
 
     if (isStartOrEndOfTransition)
     {
@@ -409,9 +408,10 @@ static Status SetCurrentLevelQuietReport(EndpointId endpoint, EmberAfLevelContro
         dirtyState     = state->quietCurrentLevel.SetValue(newValue, now, predicate);
     }
 
+    MarkAttributeDirty markDirty = MarkAttributeDirty::kNo;
     if (dirtyState == AttributeDirtyState::kMustReport)
     {
-        markDirty = MarkAttributeDirty::kIfChanged;
+        markDirty = MarkAttributeDirty::kYes;
     }
     return Attributes::CurrentLevel::Set(endpoint, state->quietCurrentLevel.value(), markDirty);
 }
@@ -545,7 +545,7 @@ static void writeRemainingTime(EndpointId endpoint, uint16_t remainingTimeMs)
         // - kMarkDirtyOnIncrement : When the value increases.
         if (state->quietRemainingTime.SetValue(remainingTimeDs, now) == AttributeDirtyState::kMustReport)
         {
-            markDirty = MarkAttributeDirty::kIfChanged;
+            markDirty = MarkAttributeDirty::kYes;
         }
 
         Attributes::RemainingTime::Set(endpoint, state->quietRemainingTime.value().ValueOr(0), markDirty);
diff --git a/src/app/util/af-types.h b/src/app/util/af-types.h
index 929ad055d0fc1e..bd5b5ed68ce4bb 100644
--- a/src/app/util/af-types.h
+++ b/src/app/util/af-types.h
@@ -307,6 +307,11 @@ enum class MarkAttributeDirty
 {
     kIfChanged,
     kNo,
+    // kYes might need to be used if the attribute value was previously changed
+    // without reporting, and now is being set in a situation where we know
+    // reporting needs to be triggered (e.g. because QuieterReportingAttribute
+    // indicated that).
+    kYes,
 };
 
 } // namespace app
diff --git a/src/app/util/attribute-table.cpp b/src/app/util/attribute-table.cpp
index 57b5f43d35071f..303b234ff93862 100644
--- a/src/app/util/attribute-table.cpp
+++ b/src/app/util/attribute-table.cpp
@@ -407,7 +407,12 @@ Status emAfWriteAttribute(EndpointId endpoint, ClusterId cluster, AttributeId at
 
     if (!valueChanging)
     {
-        // Just do nothing.
+        // Just do nothing, except triggering reporting if forced.
+        if (markDirty == MarkAttributeDirty::kYes)
+        {
+            MatterReportingAttributeChangeCallback(endpoint, cluster, attributeID);
+        }
+
         return Status::Success;
     }