Time sync: TSC feature (#28418)

* DNS: Early draft of time sync TSC feature

* Address TODOs

- make delegates async and combine
- move define into build file
- read granularity from device

* Add test to CI

* Restyled by whitespace

* Restyled by gn

* Restyled by isort

* Linty linter

* more lint

* lint trap cleanout

* what's going on with the exit of the repl?

* Shell processes never say die.

* Remove a stale todo

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

---------

Co-authored-by: Restyled.io <commits@restyled.io>

.github/workflows/tests.yaml

@@ -458,6 +458,7 @@ jobs:
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace-to json:out/trace_data/app-{SCRIPT_BASE_NAME}.json" --script "src/python_testing/TC_RVCCLEANM_1_2.py" --script-args "--int-arg PIXIT_ENDPOINT:1 --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace-to json:out/trace_data/app-{SCRIPT_BASE_NAME}.json" --script "src/python_testing/TC_RVCRUNM_1_2.py" --script-args "--int-arg PIXIT_ENDPOINT:1 --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --script "src/python_testing/TestMatterTestingSupport.py" --script-args "--trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'
+                  scripts/run_in_python_env.sh out/venv './scripts/tests/TestTimeSyncTrustedTimeSourceRunner.py'
             - name: Uploading core files
               uses: actions/upload-artifact@v3
               if: ${{ failure() && !env.ACT }}

examples/platform/linux/AppMain.cpp

@@ -41,6 +41,7 @@
 
 #include <platform/CommissionableDataProvider.h>
 #include <platform/DiagnosticDataProvider.h>
+#include <platform/RuntimeOptionsProvider.h>
 
 #include <DeviceInfoProviderImpl.h>
 
@@ -542,6 +543,9 @@ void ChipLinuxAppMainLoop(AppMainLoopImplementation * impl)
     // We need to set DeviceInfoProvider before Server::Init to setup the storage of DeviceInfoProvider properly.
     DeviceLayer::SetDeviceInfoProvider(&gExampleDeviceInfoProvider);
 
+    chip::app::RuntimeOptionsProvider::Instance().SetSimulateNoInternalTime(
+        LinuxDeviceOptions::GetInstance().mSimulateNoInternalTime);
+
     // Init ZCL Data Model and CHIP App Server
     Server::GetInstance().Init(initParams);
 

examples/platform/linux/Options.cpp

@@ -83,6 +83,7 @@ enum
     kDeviceOption_TestEventTriggerEnableKey             = 0x101f,
     kCommissionerOption_FabricID                        = 0x1020,
     kTraceTo                                            = 0x1021,
+    kOptionSimulateNoInternalTime                       = 0x1022,
 };
 
 constexpr unsigned kAppUsageLength = 64;
@@ -136,6 +137,7 @@ OptionDef sDeviceOptionDefs[] = {
 #if ENABLE_TRACING
     { "trace-to", kArgumentRequired, kTraceTo },
 #endif
+    { "simulate-no-internal-time", kNoArgument, kOptionSimulateNoInternalTime },
     {}
 };
 
@@ -250,6 +252,8 @@ const char * sDeviceOptionHelp =
     "  --trace-to <destination>\n"
     "       Trace destinations, comma separated (" SUPPORTED_COMMAND_LINE_TRACING_TARGETS ")\n"
 #endif
+    "  --simulate-no-internal-time\n"
+    "       Time cluster does not use internal platform time\n"
     "\n";
 
 bool Base64ArgToVector(const char * arg, size_t maxSize, std::vector<uint8_t> & outVector)
@@ -500,6 +504,9 @@ bool HandleOption(const char * aProgram, OptionSet * aOptions, int aIdentifier,
         LinuxDeviceOptions::GetInstance().traceTo.push_back(aValue);
         break;
 #endif
+    case kOptionSimulateNoInternalTime:
+        LinuxDeviceOptions::GetInstance().mSimulateNoInternalTime = true;
+        break;
     default:
         PrintArgError("%s: INTERNAL ERROR: Unhandled option: %s\n", aProgram, aName);
         retval = false;

examples/platform/linux/Options.h

@@ -66,6 +66,7 @@ struct LinuxDeviceOptions
     uint8_t testEventTriggerEnableKey[16] = { 0 };
     chip::FabricId commissionerFabricId   = chip::kUndefinedFabricId;
     std::vector<std::string> traceTo;
+    bool mSimulateNoInternalTime = false;
 
     static LinuxDeviceOptions & GetInstance();
 };

scripts/tests/TestTimeSyncTrustedTimeSourceRunner.py

@@ -0,0 +1,140 @@
+#!/usr/bin/env -S python3 -B
+
+# Copyright (c) 2023 Project CHIP Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import logging
+import os
+import signal
+import subprocess
+import sys
+import time
+
+DEFAULT_CHIP_ROOT = os.path.abspath(
+    os.path.join(os.path.dirname(__file__), '..', '..'))
+
+
+class TestDriver:
+    def __init__(self):
+        self.app_path = os.path.abspath(os.path.join(DEFAULT_CHIP_ROOT, 'out',
+                                                     'linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test', 'chip-all-clusters-app'))
+        self.run_python_test_path = os.path.abspath(os.path.join(os.path.dirname(__file__), 'run_python_test.py'))
+
+        self.script_path = os.path.abspath(os.path.join(
+            DEFAULT_CHIP_ROOT, 'src', 'python_testing', 'TestTimeSyncTrustedTimeSource.py'))
+        if not os.path.exists(self.app_path):
+            msg = 'chip-all-clusters-app not found'
+            logging.error(msg)
+            raise FileNotFoundError(msg)
+        if not os.path.exists(self.run_python_test_path):
+            msg = 'run_python_test.py script not found'
+            logging.error(msg)
+            raise FileNotFoundError(msg)
+        if not os.path.exists(self.script_path):
+            msg = 'TestTimeSyncTrustedTimeSource.py script not found'
+            logging.error(msg)
+            raise FileNotFoundError(msg)
+
+    def get_base_run_python_cmd(self, run_python_test_path, app_path, app_args, script_path, script_args):
+        return f'{str(run_python_test_path)} --app {str(app_path)} --app-args "{app_args}" --script {str(script_path)} --script-args "{script_args}"'
+
+    def run_test_section(self, app_args: str, script_args: str, factory_reset_all: bool = False, factory_reset_app: bool = False) -> int:
+        # quotes are required here
+        cmd = self.get_base_run_python_cmd(self.run_python_test_path, self.app_path, app_args,
+                                           self.script_path, script_args)
+        if factory_reset_all:
+            cmd = cmd + ' --factoryreset'
+        if factory_reset_app:
+            cmd = cmd + ' --factoryreset-app-only'
+
+        logging.info(f'Running cmd {cmd}')
+
+        process = subprocess.Popen(cmd, stdout=sys.stdout, stderr=sys.stderr, shell=True, bufsize=1)
+
+        return process.wait()
+
+
+def kill_process(app2_process):
+    logging.warning("Stopping app with SIGINT")
+    app2_process.send_signal(signal.SIGINT.value)
+    app2_process.wait()
+
+
+def main():
+    # in the first round, we're just going to commission the device
+    base_app_args = '--discriminator 1234 --KVS kvs1'
+    app_args = base_app_args
+    base_script_args = '--storage-path admin_storage.json --discriminator 1234 --passcode 20202021'
+    script_args = base_script_args + ' --commissioning-method on-network --commission-only'
+
+    driver = TestDriver()
+    ret = driver.run_test_section(app_args, script_args, factory_reset_all=True)
+    if ret != 0:
+        return ret
+
+    # For this test, we need to have a time source set up already for the simulated no-internal-time source to query.
+    # This means it needs to be commissioned onto the same fabric, and the ACLs need to be set up to allow
+    # access to the time source cluster.
+    # This simulates the second device, so its using a different KVS and nodeid, which will allow both apps to run simultaneously
+    app2_args = '--discriminator 1235 --KVS kvs2 --secured-device-port 5580'
+    script_args = '--storage-path admin_storage.json --discriminator 1235 --passcode 20202021 --commissioning-method on-network --dut-node-id 2 --tests test_SetupTimeSourceACL'
+
+    ret = driver.run_test_section(app2_args, script_args, factory_reset_app=True)
+    if ret != 0:
+        return ret
+
+    # Now we've got something commissioned, we're going to test what happens when it resets, but we're simulating no time.
+    # In this case, the commissioner hasn't set the time after the reboot, so there should be no time returned (checked in test)
+    app_args = base_app_args + ' --simulate-no-internal-time'
+    script_args = base_script_args + ' --tests test_SimulateNoInternalTime'
+
+    ret = driver.run_test_section(app_args, script_args)
+    if ret != 0:
+        return ret
+
+    # Make sure we come up with internal time correctly if we don't set that flag
+    app_args = base_app_args
+    script_args = base_script_args + ' --tests test_HaveInternalTime'
+
+    ret = driver.run_test_section(app_args, script_args)
+    if ret != 0:
+        return ret
+
+    # Bring up app2 again, it needs to run for the duration of the next test so app1 has a place to query time
+    # App1 will come up, it is simulating having no internal time (confirmed in previous test), but we have
+    # set up app2 as the trusted time source, so it should query out to app2 for the time.
+    app2_cmd = str(driver.app_path) + ' ' + app2_args
+    app2_process = subprocess.Popen(app2_cmd.split(), stdout=sys.stdout, stderr=sys.stderr, bufsize=0)
+
+    # Give app2 a second to come up and start advertising
+    time.sleep(1)
+
+    # This first test ensures that we read from the trusted time source right after it is set.
+    app_args = base_app_args + ' --simulate-no-internal-time --trace_decode 1'
+    script_args = base_script_args + ' --tests test_SetAndReadFromTrustedTimeSource --int-arg trusted_time_source:2'
+    ret = driver.run_test_section(app_args, script_args)
+    if ret != 0:
+        kill_process(app2_process)
+        return ret
+
+    # This next test ensures the trusted time source is saved during a reboot
+    script_args = base_script_args + ' --tests test_ReadFromTrustedTimeSource'
+    ret = driver.run_test_section(app_args, script_args)
+
+    kill_process(app2_process)
+    sys.exit(ret)
+
+
+if __name__ == '__main__':
+    main()

scripts/tests/run_python_test.py

@@ -72,6 +72,8 @@ def DumpProgramOutputToQueue(thread_list: typing.List[threading.Thread], tag: st
               help='Path to local application to use, omit to use external apps.')
 @click.option("--factoryreset", is_flag=True,
               help='Remove app config and repl configs (/tmp/chip* and /tmp/repl*) before running the tests.')
+@click.option("--factoryreset-app-only", is_flag=True,
+              help='Remove app config and repl configs (/tmp/chip* and /tmp/repl*) before running the tests, but not the controller config')
 @click.option("--app-args", type=str, default='',
               help='The extra arguments passed to the device. Can use placholders like {SCRIPT_BASE_NAME}')
 @click.option("--script", type=click.Path(exists=True), default=os.path.join(DEFAULT_CHIP_ROOT,
@@ -85,11 +87,11 @@ def DumpProgramOutputToQueue(thread_list: typing.List[threading.Thread], tag: st
               help='Script arguments, can use placeholders like {SCRIPT_BASE_NAME}.')
 @click.option("--script-gdb", is_flag=True,
               help='Run script through gdb')
-def main(app: str, factoryreset: bool, app_args: str, script: str, script_args: str, script_gdb: bool):
+def main(app: str, factoryreset: bool, factoryreset_app_only: bool, app_args: str, script: str, script_args: str, script_gdb: bool):
     app_args = app_args.replace('{SCRIPT_BASE_NAME}', os.path.splitext(os.path.basename(script))[0])
     script_args = script_args.replace('{SCRIPT_BASE_NAME}', os.path.splitext(os.path.basename(script))[0])
 
-    if factoryreset:
+    if factoryreset or factoryreset_app_only:
         # Remove native app config
         retcode = subprocess.call("rm -rf /tmp/chip* /tmp/repl*", shell=True)
         if retcode != 0:
@@ -107,6 +109,7 @@ def main(app: str, factoryreset: bool, app_args: str, script: str, script_args:
             if retcode != 0:
                 raise Exception("Failed to remove %s for factory reset." % kvs_path_to_remove)
 
+    if factoryreset:
         # Remove Python test admin storage if provided
         storage_match = re.search(r"--storage-path (?P<storage_path>[^ ]+)", script_args)
         if storage_match:

src/app/chip_data_model.gni

@@ -42,6 +42,7 @@ template("chip_data_model") {
     # Allow building ota-requestor-app with a non-spec-compliant floor
     # (i.e. smaller than 2 minutes) for action delays.
     non_spec_compliant_ota_action_delay_floor = -1
+    time_sync_enable_tsc_feature = 1
   }
 
   if (defined(invoker.idl)) {
@@ -198,6 +199,10 @@ template("chip_data_model") {
       deps = []
     }
 
+    if (!defined(cflags)) {
+      cflags = []
+    }
+
     foreach(cluster, _cluster_sources) {
       if (cluster == "door-lock-server") {
         sources += [
@@ -257,6 +262,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}" ]
       } else if (cluster == "scenes-server") {
         sources += [
           "${_app_root}/clusters/${cluster}/${cluster}.cpp",
@@ -318,10 +325,6 @@ template("chip_data_model") {
       public_deps += [ "${chip_root}/src/app/server" ]
     }
 
-    if (!defined(cflags)) {
-      cflags = []
-    }
-
     cflags += [ "-Wconversion" ]
 
     if (non_spec_compliant_ota_action_delay_floor >= 0) {

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

@@ -18,6 +18,8 @@
 
 #include "DefaultTimeSyncDelegate.h"
 #include "inet/IPAddress.h"
+#include <platform/RuntimeOptionsProvider.h>
+#include <system/SystemClock.h>
 
 using chip::TimeSyncDataProvider;
 using namespace chip::app::Clusters::TimeSynchronization;
@@ -45,3 +47,27 @@ bool DefaultTimeSyncDelegate::IsNTPAddressDomain(chip::CharSpan ntp)
     // placeholder implementation
     return false;
 }
+
+CHIP_ERROR DefaultTimeSyncDelegate::UpdateTimeFromPlatformSource(chip::Callback::Callback<OnTimeSyncCompletion> * callback)
+{
+    System::Clock::Microseconds64 utcTime;
+    if (chip::app::RuntimeOptionsProvider::Instance().GetSimulateNoInternalTime())
+    {
+        return CHIP_ERROR_NOT_IMPLEMENTED;
+    }
+    if (System::SystemClock().GetClock_RealTime(utcTime) == CHIP_NO_ERROR)
+    {
+        // Default assumes the time came from NTP. Platforms using other sources should overwrite this
+        // with their own delegates
+        // Call the callback right away from within this function
+        callback->mCall(callback->mContext, TimeSourceEnum::kMixedNTP, GranularityEnum::kMillisecondsGranularity);
+        return CHIP_NO_ERROR;
+    }
+    return CHIP_ERROR_NOT_IMPLEMENTED;
+}
+
+CHIP_ERROR DefaultTimeSyncDelegate::UpdateTimeUsingNTPFallback(const CharSpan & fallbackNTP,
+                                                               chip::Callback::Callback<OnFallbackNTPCompletion> * callback)
+{
+    return CHIP_ERROR_NOT_IMPLEMENTED;
+}

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

@@ -33,6 +33,9 @@ class DefaultTimeSyncDelegate : public Delegate
     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;
 };
 
 } // namespace TimeSynchronization

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

@@ -29,6 +29,9 @@ namespace app {
 namespace Clusters {
 namespace TimeSynchronization {
 
+typedef void (*OnTimeSyncCompletion)(void * context, TimeSourceEnum timeSource, GranularityEnum granularity);
+typedef void (*OnFallbackNTPCompletion)(void * context, bool timeSyncSuccessful);
+
 /** @brief
  *    Defines methods for implementing application-specific logic for the Time Synchronization Cluster.
  */
@@ -74,6 +77,30 @@ class Delegate
      */
     virtual bool IsNTPAddressDomain(const CharSpan ntp) = 0;
 
+    /**
+     * @brief Delegate should attempt to get time from a platform-defined source using the ordering defined in the
+     * Time source prioritization spec section. Delegate may skip any unsupported sources
+     * Order: GNSS -> trusted high-resolution external source (PTP, trusted network NTP, cloud) ->
+     * 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)
+     * 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.
+     */
+    virtual CHIP_ERROR UpdateTimeFromPlatformSource(chip::Callback::Callback<OnTimeSyncCompletion> * callback) = 0;
+
+    /**
+     * @brief If the delegate supports NTP, it should attempt to update its time using the provided fallbackNTP source.
+     * If the delegate is successful in obtaining a time from the fallbackNTP, it updates the system time (ex using
+     * System::SystemClock().SetClock_RealTime) and calls the callback. If the delegate is going to attempt to update
+     * the time and call the callback, it returns CHIP_NO_ERROR. If the delegate does not support NTP, it may return
+     * a CHIP_ERROR.
+     */
+    virtual CHIP_ERROR UpdateTimeUsingNTPFallback(const CharSpan & fallbackNTP,
+                                                  chip::Callback::Callback<OnFallbackNTPCompletion> * callback) = 0;
+
     virtual ~Delegate() = default;
 
 private: