Time sync: Remove ClusterStateCache (#29396)

* Time sync: Remove ClusterStateCache

It's easy to use, but it sure is big. Since we aren't asking for
lists or anything, it's also overkill.

* pack all the things into a class.

* Fiddle with the build file

* Comments on the delegate.

* remove one more app prefix

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

---------

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

Author: 2 people

src/app/chip_data_model.gni

@@ -203,6 +203,10 @@ template("chip_data_model") {
       cflags = []
     }
 
+    if (!defined(defines)) {
+      defines = []
+    }
+
     foreach(cluster, _cluster_sources) {
       if (cluster == "door-lock-server") {
         sources += [
@@ -262,8 +266,8 @@ template("chip_data_model") {
           "${_app_root}/clusters/${cluster}/DefaultTimeSyncDelegate.cpp",
           "${_app_root}/clusters/${cluster}/TimeSyncDataProvider.cpp",
         ]
-        cflags +=
-            [ "-DTIME_SYNC_ENABLE_TSC_FEATURE=${time_sync_enable_tsc_feature}" ]
+        defines +=
+            [ "TIME_SYNC_ENABLE_TSC_FEATURE=${time_sync_enable_tsc_feature}" ]
       } else if (cluster == "scenes-server") {
         sources += [
           "${_app_root}/clusters/${cluster}/${cluster}.cpp",

src/app/clusters/time-synchronization-server/time-synchronization-delegate.h

@@ -84,7 +84,11 @@ class Delegate
      * local network defined NTP (DHCPv6 -> DHCP -> DNS-SD sources)
      * If the delegate is unable to support any source, it may return an error immediately. If the delegate is going
      * to attempt to obtain time from any source, it returns CHIP_NO_ERROR and calls the callback on completion.
-     * If the delegate successfully obtains the time, it sets the time using the platform time API (SetClock_RealTime)
+     * If the delegate has a time available at the time of this call, it may call the callback synchronously from within
+     * this function.
+     * If the delegate needs to reach out asynchronously to obtain a time, it saves this callback to call asynchronously.
+     * The delegate should track these callbacks in a CallbackDeque to ensure they can be properly cancelled.
+     * If the delegate is successful in obtaining the time, it sets the time using the platform time API (SetClock_RealTime)
      * and calls the callback with the time source and granularity set as appropriate.
      * If the delegate is unsuccessful in obtaining the time, it calls the callback with timeSource set to kNone and
      * granularity set to kNoTimeGranularity.

src/app/clusters/time-synchronization-server/time-synchronization-server.cpp

@@ -300,41 +300,30 @@ void TimeSynchronizationServer::AttemptToGetFallbackNTPTimeFromDelegate()
 void TimeSynchronizationServer::OnDeviceConnectedFn(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle)
 {
     // Connected to our trusted time source, let's read the time.
-    app::AttributePathParams readPaths[2];
-    readPaths[0] = app::AttributePathParams(kRootEndpointId, app::Clusters::TimeSynchronization::Id,
-                                            app::Clusters::TimeSynchronization::Attributes::UTCTime::Id);
-    readPaths[1] = app::AttributePathParams(kRootEndpointId, app::Clusters::TimeSynchronization::Id,
-                                            app::Clusters::TimeSynchronization::Attributes::Granularity::Id);
-
-    app::InteractionModelEngine * engine = app::InteractionModelEngine::GetInstance();
-    app::ReadPrepareParams readParams(sessionHandle);
+    AttributePathParams readPaths[2];
+    readPaths[0] = AttributePathParams(kRootEndpointId, Id, Attributes::UTCTime::Id);
+    readPaths[1] = AttributePathParams(kRootEndpointId, Id, Attributes::Granularity::Id);
+
+    InteractionModelEngine * engine = InteractionModelEngine::GetInstance();
+    ReadPrepareParams readParams(sessionHandle);
     readParams.mpAttributePathParamsList    = readPaths;
     readParams.mAttributePathParamsListSize = 2;
 
-    auto attributeCache = Platform::MakeUnique<app::ClusterStateCache>(*this);
-    if (attributeCache == nullptr)
+    auto readInfo = Platform::MakeUnique<TimeReadInfo>(engine, &exchangeMgr, *this, ReadClient::InteractionType::Read);
+    if (readInfo == nullptr)
     {
         // This is unlikely to work if we don't have memory, but let's try
         OnDeviceConnectionFailureFn();
         return;
     }
-    auto readClient = chip::Platform::MakeUnique<app::ReadClient>(engine, &exchangeMgr, attributeCache->GetBufferedCallback(),
-                                                                  app::ReadClient::InteractionType::Read);
-    if (readClient == nullptr)
-    {
-        // This is unlikely to work if we don't have memory, but let's try
-        OnDeviceConnectionFailureFn();
-        return;
-    }
-    CHIP_ERROR err = readClient->SendRequest(readParams);
+    CHIP_ERROR err = readInfo->readClient.SendRequest(readParams);
     if (err != CHIP_NO_ERROR)
     {
         ChipLogError(Zcl, "Failed to read UTC time from trusted source");
         OnDeviceConnectionFailureFn();
         return;
     }
-    mAttributeCache = std::move(attributeCache);
-    mReadClient     = std::move(readClient);
+    mTimeReadInfo = std::move(readInfo);
 }
 
 void TimeSynchronizationServer::OnDeviceConnectionFailureFn()
@@ -343,20 +332,39 @@ void TimeSynchronizationServer::OnDeviceConnectionFailureFn()
     AttemptToGetFallbackNTPTimeFromDelegate();
 }
 
-void TimeSynchronizationServer::OnDone(ReadClient * apReadClient)
+void TimeSynchronizationServer::OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData,
+                                                const StatusIB & aStatus)
 {
-    using namespace chip::app::Clusters::TimeSynchronization::Attributes;
-
-    Granularity::TypeInfo::Type granularity = GranularityEnum::kNoTimeGranularity;
-    mAttributeCache->Get<Granularity::TypeInfo>(kRootEndpointId, granularity);
+    if (aPath.mClusterId != Id || aStatus.IsFailure())
+    {
+        return;
+    }
+    switch (aPath.mAttributeId)
+    {
+    case Attributes::UTCTime::Id:
+        if (DataModel::Decode(*apData, mTimeReadInfo->utcTime) != CHIP_NO_ERROR)
+        {
+            mTimeReadInfo->utcTime.SetNull();
+        }
+        break;
+    case Attributes::Granularity::Id:
+        if (DataModel::Decode(*apData, mTimeReadInfo->granularity) != CHIP_NO_ERROR)
+        {
+            mTimeReadInfo->granularity = GranularityEnum::kNoTimeGranularity;
+        }
+        break;
+    default:
+        break;
+    }
+}
 
-    UTCTime::TypeInfo::Type time;
-    CHIP_ERROR err = mAttributeCache->Get<UTCTime::TypeInfo>(kRootEndpointId, time);
-    if (err == CHIP_NO_ERROR && !time.IsNull() && granularity != GranularityEnum::kNoTimeGranularity)
+void TimeSynchronizationServer::OnDone(ReadClient * apReadClient)
+{
+    if (!mTimeReadInfo->utcTime.IsNull() && mTimeReadInfo->granularity != GranularityEnum::kNoTimeGranularity)
     {
         GranularityEnum ourGranularity;
         // Being conservative with granularity - nothing smaller than seconds because of network delay
-        switch (granularity)
+        switch (mTimeReadInfo->granularity)
         {
         case GranularityEnum::kMinutesGranularity:
         case GranularityEnum::kSecondsGranularity:
@@ -367,7 +375,8 @@ void TimeSynchronizationServer::OnDone(ReadClient * apReadClient)
             break;
         }
 
-        err = SetUTCTime(kRootEndpointId, time.Value(), ourGranularity, TimeSourceEnum::kNodeTimeCluster);
+        CHIP_ERROR err =
+            SetUTCTime(kRootEndpointId, mTimeReadInfo->utcTime.Value(), ourGranularity, TimeSourceEnum::kNodeTimeCluster);
         if (err == CHIP_NO_ERROR)
         {
             return;
@@ -377,6 +386,7 @@ void TimeSynchronizationServer::OnDone(ReadClient * apReadClient)
     // If we failed to set the UTC time, it doesn't hurt to try the backup - NTP system might have different permissions on the
     // system clock
     AttemptToGetFallbackNTPTimeFromDelegate();
+    mTimeReadInfo = nullptr;
 }
 #endif
 

src/app/clusters/time-synchronization-server/time-synchronization-server.h

@@ -72,7 +72,7 @@ enum class TimeSyncEventFlag : uint8_t
 class TimeSynchronizationServer : public FabricTable::Delegate
 #if TIME_SYNC_ENABLE_TSC_FEATURE
     ,
-                                  public ClusterStateCache::Callback
+                                  public ReadClient::Callback
 #endif
 {
 public:
@@ -117,8 +117,8 @@ class TimeSynchronizationServer : public FabricTable::Delegate
     void OnDeviceConnectedFn(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle);
     void OnDeviceConnectionFailureFn();
 
-    // AttributeCache::Callback functions
-    void OnAttributeChanged(ClusterStateCache * cache, const ConcreteAttributePath & path) override {}
+    // ReadClient::Callback functions
+    void OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) override;
     void OnDone(ReadClient * apReadClient) override;
 #endif
 
@@ -142,10 +142,21 @@ class TimeSynchronizationServer : public FabricTable::Delegate
     static TimeSynchronizationServer sTimeSyncInstance;
     TimeSyncEventFlag mEventFlag = TimeSyncEventFlag::kNone;
 #if TIME_SYNC_ENABLE_TSC_FEATURE
-    Platform::UniquePtr<app::ClusterStateCache> mAttributeCache;
-    Platform::UniquePtr<app::ReadClient> mReadClient;
     chip::Callback::Callback<chip::OnDeviceConnected> mOnDeviceConnectedCallback;
     chip::Callback::Callback<chip::OnDeviceConnectionFailure> mOnDeviceConnectionFailureCallback;
+    struct TimeReadInfo
+    {
+        TimeReadInfo(InteractionModelEngine * apImEngine, Messaging::ExchangeManager * apExchangeMgr,
+                     ReadClient::Callback & apCallback, ReadClient::InteractionType aInteractionType) :
+            readClient(apImEngine, apExchangeMgr, apCallback, aInteractionType)
+        {
+            utcTime.SetNull();
+        }
+        Attributes::UTCTime::TypeInfo::DecodableType utcTime;
+        Attributes::Granularity::TypeInfo::DecodableType granularity = GranularityEnum::kNoTimeGranularity;
+        ReadClient readClient;
+    };
+    Platform::UniquePtr<TimeReadInfo> mTimeReadInfo;
 #endif
     chip::Callback::Callback<OnTimeSyncCompletion> mOnTimeSyncCompletion;
     chip::Callback::Callback<OnFallbackNTPCompletion> mOnFallbackNTPCompletion;