Timesync improvements - part 1 (#32848)

* change cluster revision in light-switch-app

* added delegate functions for better usability

* - made AttemptToGetTimeFromTrustedNode() public API
- free readclient memory after read is complete

* added default implementation for some delegate functions

* added documentation for delegate functions

Author: fessehaeve

examples/light-switch-app/light-switch-common/light-switch-app.matter

@@ -2792,7 +2792,7 @@ endpoint 0 {
     callback attribute acceptedCommandList;
     callback attribute attributeList;
     ram      attribute featureMap default = 0x0B;
-    ram      attribute clusterRevision default = 1;
+    ram      attribute clusterRevision default = 2;
 
     handle command SetUTCTime;
     handle command SetTrustedTimeSource;

examples/light-switch-app/light-switch-common/light-switch-app.zap

@@ -3826,7 +3826,7 @@
               "storageOption": "RAM",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "1",
+              "defaultValue": "2",
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,

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

@@ -24,17 +24,6 @@
 using chip::TimeSyncDataProvider;
 using namespace chip::app::Clusters::TimeSynchronization;
 
-void DefaultTimeSyncDelegate::TimeZoneListChanged(const Span<TimeSyncDataProvider::TimeZoneStore> timeZoneList)
-{
-    // placeholder implementation
-}
-
-bool DefaultTimeSyncDelegate::HandleUpdateDSTOffset(chip::CharSpan name)
-{
-    // placeholder implementation
-    return false;
-}
-
 bool DefaultTimeSyncDelegate::IsNTPAddressValid(chip::CharSpan ntp)
 {
     // placeholder implementation
@@ -67,14 +56,3 @@ CHIP_ERROR DefaultTimeSyncDelegate::UpdateTimeFromPlatformSource(chip::Callback:
     }
     return CHIP_ERROR_NOT_IMPLEMENTED;
 }
-
-CHIP_ERROR DefaultTimeSyncDelegate::UpdateTimeUsingNTPFallback(const CharSpan & fallbackNTP,
-                                                               chip::Callback::Callback<OnFallbackNTPCompletion> * callback)
-{
-    return CHIP_ERROR_NOT_IMPLEMENTED;
-}
-
-void DefaultTimeSyncDelegate::UTCTimeAvailabilityChanged(uint64_t time)
-{
-    // placeholder implementation
-}

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

@@ -29,14 +29,9 @@ class DefaultTimeSyncDelegate : public Delegate
 
 public:
     DefaultTimeSyncDelegate() : Delegate(){};
-    void TimeZoneListChanged(const Span<TimeSyncDataProvider::TimeZoneStore> timeZoneList) override;
-    bool HandleUpdateDSTOffset(CharSpan name) override;
     bool IsNTPAddressValid(CharSpan ntp) override;
     bool IsNTPAddressDomain(CharSpan ntp) override;
     CHIP_ERROR UpdateTimeFromPlatformSource(chip::Callback::Callback<OnTimeSyncCompletion> * callback) override;
-    CHIP_ERROR UpdateTimeUsingNTPFallback(const CharSpan & fallbackNTP,
-                                          chip::Callback::Callback<OnFallbackNTPCompletion> * callback) override;
-    void UTCTimeAvailabilityChanged(uint64_t time) override;
 };
 
 } // namespace TimeSynchronization

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

@@ -56,15 +56,15 @@ class Delegate
      *
      * @param timeZoneList new time zone list
      */
-    virtual void TimeZoneListChanged(const Span<TimeSyncDataProvider::TimeZoneStore> timeZoneList) = 0;
+    virtual void TimeZoneListChanged(const Span<TimeSyncDataProvider::TimeZoneStore> timeZoneList) {}
     /**
      * @brief Give the delegate the chance to call SetDSTOffset on the TimeSynchronizationServer with a list of
      * DST offsets based on the provided time zone name.  If the delegate does so, it should return true.
      * If the delegate does not want to set DST offsets based on the time zone, it should return false.
      *
      * @param name name of active time zone
      */
-    virtual bool HandleUpdateDSTOffset(const CharSpan name) = 0;
+    virtual bool HandleUpdateDSTOffset(const CharSpan name) { return false; }
     /**
      * @brief Returns true if the provided string is a valid NTP address (either domain name or IPv6 address).
      *
@@ -104,12 +104,26 @@ class Delegate
      * a CHIP_ERROR.
      */
     virtual CHIP_ERROR UpdateTimeUsingNTPFallback(const CharSpan & fallbackNTP,
-                                                  chip::Callback::Callback<OnFallbackNTPCompletion> * callback) = 0;
+                                                  chip::Callback::Callback<OnFallbackNTPCompletion> * callback)
+    {
+        return CHIP_ERROR_NOT_IMPLEMENTED;
+    }
 
     /**
-     * @brief Signals application that UTCTime has changed through the timesync cluster.
+     * @brief Signals application that UTCTime has changed through the timesync cluster. This gets called when
+     * time is available for the first time or is updated. Therefore, @param time will always have a valid value.
+     * The negative case of time being unavailable is handled by NotifyTimeFailure().
+     */
+    virtual void UTCTimeAvailabilityChanged(uint64_t time) {}
+    /**
+     * @brief Signals application that a new trusted time source is available. The application can then decide
+     * if it wants to attempt to query for time from this source.
+     */
+    virtual void TrustedTimeSourceAvailabilityChanged(bool available, GranularityEnum granularity) {}
+    /**
+     * @brief Signals application that fetching time has failed. The reason is not relevant.
      */
-    virtual void UTCTimeAvailabilityChanged(uint64_t time) = 0;
+    virtual void NotifyTimeFailure() {}
 
     virtual ~Delegate() = default;
 

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

@@ -215,6 +215,7 @@ static bool emitTimeFailureEvent(EndpointId ep)
     // TODO: re-schedule event for after min 1hr if no time is still available
     // https://github.com/project-chip/connectedhomeip/issues/27200
     ChipLogProgress(Zcl, "Emit TimeFailure event [ep=%d]", ep);
+    GetDelegate()->NotifyTimeFailure();
     return true;
 }
 
@@ -356,6 +357,7 @@ void TimeSynchronizationServer::OnDone(ReadClient * apReadClient)
             SetUTCTime(kRootEndpointId, mTimeReadInfo->utcTime.Value(), ourGranularity, TimeSourceEnum::kNodeTimeCluster);
         if (err == CHIP_NO_ERROR)
         {
+            mTimeReadInfo = nullptr;
             return;
         }
     }
@@ -504,6 +506,7 @@ CHIP_ERROR TimeSynchronizationServer::SetTrustedTimeSource(const DataModel::Null
     {
         AttemptToGetTime();
     }
+    GetDelegate()->TrustedTimeSourceAvailabilityChanged(!mTrustedTimeSource.IsNull(), mGranularity);
     return err;
 }
 

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

@@ -123,6 +123,8 @@ class TimeSynchronizationServer : public FabricTable::Delegate
     // ReadClient::Callback functions
     void OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) override;
     void OnDone(ReadClient * apReadClient) override;
+
+    CHIP_ERROR AttemptToGetTimeFromTrustedNode();
 #endif
 
     // Platform event handler functions
@@ -166,7 +168,6 @@ class TimeSynchronizationServer : public FabricTable::Delegate
 
     // Called when the platform is set up - attempts to get time using the recommended source list in the spec.
     void AttemptToGetTime();
-    CHIP_ERROR AttemptToGetTimeFromTrustedNode();
     // Attempts to get fallback NTP from the delegate (last available source)
     // If successful, the function will set mGranulatiry and the time source
     // If unsuccessful, it will emit a TimeFailure event.