Skip to content

Commit 2e97ec0

Browse files
Time sync: TSC feature (project-chip#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>
1 parent 13f99a5 commit 2e97ec0

16 files changed

+712
-20
lines changed

.github/workflows/tests.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,7 @@ jobs:
458458
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"'
459459
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"'
460460
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"'
461+
scripts/run_in_python_env.sh out/venv './scripts/tests/TestTimeSyncTrustedTimeSourceRunner.py'
461462
- name: Uploading core files
462463
uses: actions/upload-artifact@v3
463464
if: ${{ failure() && !env.ACT }}

examples/platform/linux/AppMain.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141

4242
#include <platform/CommissionableDataProvider.h>
4343
#include <platform/DiagnosticDataProvider.h>
44+
#include <platform/RuntimeOptionsProvider.h>
4445

4546
#include <DeviceInfoProviderImpl.h>
4647

@@ -542,6 +543,9 @@ void ChipLinuxAppMainLoop(AppMainLoopImplementation * impl)
542543
// We need to set DeviceInfoProvider before Server::Init to setup the storage of DeviceInfoProvider properly.
543544
DeviceLayer::SetDeviceInfoProvider(&gExampleDeviceInfoProvider);
544545

546+
chip::app::RuntimeOptionsProvider::Instance().SetSimulateNoInternalTime(
547+
LinuxDeviceOptions::GetInstance().mSimulateNoInternalTime);
548+
545549
// Init ZCL Data Model and CHIP App Server
546550
Server::GetInstance().Init(initParams);
547551

examples/platform/linux/Options.cpp

+7
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ enum
8383
kDeviceOption_TestEventTriggerEnableKey = 0x101f,
8484
kCommissionerOption_FabricID = 0x1020,
8585
kTraceTo = 0x1021,
86+
kOptionSimulateNoInternalTime = 0x1022,
8687
};
8788

8889
constexpr unsigned kAppUsageLength = 64;
@@ -136,6 +137,7 @@ OptionDef sDeviceOptionDefs[] = {
136137
#if ENABLE_TRACING
137138
{ "trace-to", kArgumentRequired, kTraceTo },
138139
#endif
140+
{ "simulate-no-internal-time", kNoArgument, kOptionSimulateNoInternalTime },
139141
{}
140142
};
141143

@@ -250,6 +252,8 @@ const char * sDeviceOptionHelp =
250252
" --trace-to <destination>\n"
251253
" Trace destinations, comma separated (" SUPPORTED_COMMAND_LINE_TRACING_TARGETS ")\n"
252254
#endif
255+
" --simulate-no-internal-time\n"
256+
" Time cluster does not use internal platform time\n"
253257
"\n";
254258

255259
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,
500504
LinuxDeviceOptions::GetInstance().traceTo.push_back(aValue);
501505
break;
502506
#endif
507+
case kOptionSimulateNoInternalTime:
508+
LinuxDeviceOptions::GetInstance().mSimulateNoInternalTime = true;
509+
break;
503510
default:
504511
PrintArgError("%s: INTERNAL ERROR: Unhandled option: %s\n", aProgram, aName);
505512
retval = false;

examples/platform/linux/Options.h

+1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ struct LinuxDeviceOptions
6666
uint8_t testEventTriggerEnableKey[16] = { 0 };
6767
chip::FabricId commissionerFabricId = chip::kUndefinedFabricId;
6868
std::vector<std::string> traceTo;
69+
bool mSimulateNoInternalTime = false;
6970

7071
static LinuxDeviceOptions & GetInstance();
7172
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
#!/usr/bin/env -S python3 -B
2+
3+
# Copyright (c) 2023 Project CHIP Authors
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
import logging
18+
import os
19+
import signal
20+
import subprocess
21+
import sys
22+
import time
23+
24+
DEFAULT_CHIP_ROOT = os.path.abspath(
25+
os.path.join(os.path.dirname(__file__), '..', '..'))
26+
27+
28+
class TestDriver:
29+
def __init__(self):
30+
self.app_path = os.path.abspath(os.path.join(DEFAULT_CHIP_ROOT, 'out',
31+
'linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test', 'chip-all-clusters-app'))
32+
self.run_python_test_path = os.path.abspath(os.path.join(os.path.dirname(__file__), 'run_python_test.py'))
33+
34+
self.script_path = os.path.abspath(os.path.join(
35+
DEFAULT_CHIP_ROOT, 'src', 'python_testing', 'TestTimeSyncTrustedTimeSource.py'))
36+
if not os.path.exists(self.app_path):
37+
msg = 'chip-all-clusters-app not found'
38+
logging.error(msg)
39+
raise FileNotFoundError(msg)
40+
if not os.path.exists(self.run_python_test_path):
41+
msg = 'run_python_test.py script not found'
42+
logging.error(msg)
43+
raise FileNotFoundError(msg)
44+
if not os.path.exists(self.script_path):
45+
msg = 'TestTimeSyncTrustedTimeSource.py script not found'
46+
logging.error(msg)
47+
raise FileNotFoundError(msg)
48+
49+
def get_base_run_python_cmd(self, run_python_test_path, app_path, app_args, script_path, script_args):
50+
return f'{str(run_python_test_path)} --app {str(app_path)} --app-args "{app_args}" --script {str(script_path)} --script-args "{script_args}"'
51+
52+
def run_test_section(self, app_args: str, script_args: str, factory_reset_all: bool = False, factory_reset_app: bool = False) -> int:
53+
# quotes are required here
54+
cmd = self.get_base_run_python_cmd(self.run_python_test_path, self.app_path, app_args,
55+
self.script_path, script_args)
56+
if factory_reset_all:
57+
cmd = cmd + ' --factoryreset'
58+
if factory_reset_app:
59+
cmd = cmd + ' --factoryreset-app-only'
60+
61+
logging.info(f'Running cmd {cmd}')
62+
63+
process = subprocess.Popen(cmd, stdout=sys.stdout, stderr=sys.stderr, shell=True, bufsize=1)
64+
65+
return process.wait()
66+
67+
68+
def kill_process(app2_process):
69+
logging.warning("Stopping app with SIGINT")
70+
app2_process.send_signal(signal.SIGINT.value)
71+
app2_process.wait()
72+
73+
74+
def main():
75+
# in the first round, we're just going to commission the device
76+
base_app_args = '--discriminator 1234 --KVS kvs1'
77+
app_args = base_app_args
78+
base_script_args = '--storage-path admin_storage.json --discriminator 1234 --passcode 20202021'
79+
script_args = base_script_args + ' --commissioning-method on-network --commission-only'
80+
81+
driver = TestDriver()
82+
ret = driver.run_test_section(app_args, script_args, factory_reset_all=True)
83+
if ret != 0:
84+
return ret
85+
86+
# For this test, we need to have a time source set up already for the simulated no-internal-time source to query.
87+
# This means it needs to be commissioned onto the same fabric, and the ACLs need to be set up to allow
88+
# access to the time source cluster.
89+
# This simulates the second device, so its using a different KVS and nodeid, which will allow both apps to run simultaneously
90+
app2_args = '--discriminator 1235 --KVS kvs2 --secured-device-port 5580'
91+
script_args = '--storage-path admin_storage.json --discriminator 1235 --passcode 20202021 --commissioning-method on-network --dut-node-id 2 --tests test_SetupTimeSourceACL'
92+
93+
ret = driver.run_test_section(app2_args, script_args, factory_reset_app=True)
94+
if ret != 0:
95+
return ret
96+
97+
# Now we've got something commissioned, we're going to test what happens when it resets, but we're simulating no time.
98+
# In this case, the commissioner hasn't set the time after the reboot, so there should be no time returned (checked in test)
99+
app_args = base_app_args + ' --simulate-no-internal-time'
100+
script_args = base_script_args + ' --tests test_SimulateNoInternalTime'
101+
102+
ret = driver.run_test_section(app_args, script_args)
103+
if ret != 0:
104+
return ret
105+
106+
# Make sure we come up with internal time correctly if we don't set that flag
107+
app_args = base_app_args
108+
script_args = base_script_args + ' --tests test_HaveInternalTime'
109+
110+
ret = driver.run_test_section(app_args, script_args)
111+
if ret != 0:
112+
return ret
113+
114+
# Bring up app2 again, it needs to run for the duration of the next test so app1 has a place to query time
115+
# App1 will come up, it is simulating having no internal time (confirmed in previous test), but we have
116+
# set up app2 as the trusted time source, so it should query out to app2 for the time.
117+
app2_cmd = str(driver.app_path) + ' ' + app2_args
118+
app2_process = subprocess.Popen(app2_cmd.split(), stdout=sys.stdout, stderr=sys.stderr, bufsize=0)
119+
120+
# Give app2 a second to come up and start advertising
121+
time.sleep(1)
122+
123+
# This first test ensures that we read from the trusted time source right after it is set.
124+
app_args = base_app_args + ' --simulate-no-internal-time --trace_decode 1'
125+
script_args = base_script_args + ' --tests test_SetAndReadFromTrustedTimeSource --int-arg trusted_time_source:2'
126+
ret = driver.run_test_section(app_args, script_args)
127+
if ret != 0:
128+
kill_process(app2_process)
129+
return ret
130+
131+
# This next test ensures the trusted time source is saved during a reboot
132+
script_args = base_script_args + ' --tests test_ReadFromTrustedTimeSource'
133+
ret = driver.run_test_section(app_args, script_args)
134+
135+
kill_process(app2_process)
136+
sys.exit(ret)
137+
138+
139+
if __name__ == '__main__':
140+
main()

scripts/tests/run_python_test.py

+5-2
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ def DumpProgramOutputToQueue(thread_list: typing.List[threading.Thread], tag: st
7272
help='Path to local application to use, omit to use external apps.')
7373
@click.option("--factoryreset", is_flag=True,
7474
help='Remove app config and repl configs (/tmp/chip* and /tmp/repl*) before running the tests.')
75+
@click.option("--factoryreset-app-only", is_flag=True,
76+
help='Remove app config and repl configs (/tmp/chip* and /tmp/repl*) before running the tests, but not the controller config')
7577
@click.option("--app-args", type=str, default='',
7678
help='The extra arguments passed to the device. Can use placholders like {SCRIPT_BASE_NAME}')
7779
@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
8587
help='Script arguments, can use placeholders like {SCRIPT_BASE_NAME}.')
8688
@click.option("--script-gdb", is_flag=True,
8789
help='Run script through gdb')
88-
def main(app: str, factoryreset: bool, app_args: str, script: str, script_args: str, script_gdb: bool):
90+
def main(app: str, factoryreset: bool, factoryreset_app_only: bool, app_args: str, script: str, script_args: str, script_gdb: bool):
8991
app_args = app_args.replace('{SCRIPT_BASE_NAME}', os.path.splitext(os.path.basename(script))[0])
9092
script_args = script_args.replace('{SCRIPT_BASE_NAME}', os.path.splitext(os.path.basename(script))[0])
9193

92-
if factoryreset:
94+
if factoryreset or factoryreset_app_only:
9395
# Remove native app config
9496
retcode = subprocess.call("rm -rf /tmp/chip* /tmp/repl*", shell=True)
9597
if retcode != 0:
@@ -107,6 +109,7 @@ def main(app: str, factoryreset: bool, app_args: str, script: str, script_args:
107109
if retcode != 0:
108110
raise Exception("Failed to remove %s for factory reset." % kvs_path_to_remove)
109111

112+
if factoryreset:
110113
# Remove Python test admin storage if provided
111114
storage_match = re.search(r"--storage-path (?P<storage_path>[^ ]+)", script_args)
112115
if storage_match:

src/app/chip_data_model.gni

+7-4
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ template("chip_data_model") {
4242
# Allow building ota-requestor-app with a non-spec-compliant floor
4343
# (i.e. smaller than 2 minutes) for action delays.
4444
non_spec_compliant_ota_action_delay_floor = -1
45+
time_sync_enable_tsc_feature = 1
4546
}
4647

4748
if (defined(invoker.idl)) {
@@ -198,6 +199,10 @@ template("chip_data_model") {
198199
deps = []
199200
}
200201

202+
if (!defined(cflags)) {
203+
cflags = []
204+
}
205+
201206
foreach(cluster, _cluster_sources) {
202207
if (cluster == "door-lock-server") {
203208
sources += [
@@ -257,6 +262,8 @@ template("chip_data_model") {
257262
"${_app_root}/clusters/${cluster}/DefaultTimeSyncDelegate.cpp",
258263
"${_app_root}/clusters/${cluster}/TimeSyncDataProvider.cpp",
259264
]
265+
cflags +=
266+
[ "-DTIME_SYNC_ENABLE_TSC_FEATURE=${time_sync_enable_tsc_feature}" ]
260267
} else if (cluster == "scenes-server") {
261268
sources += [
262269
"${_app_root}/clusters/${cluster}/${cluster}.cpp",
@@ -318,10 +325,6 @@ template("chip_data_model") {
318325
public_deps += [ "${chip_root}/src/app/server" ]
319326
}
320327

321-
if (!defined(cflags)) {
322-
cflags = []
323-
}
324-
325328
cflags += [ "-Wconversion" ]
326329

327330
if (non_spec_compliant_ota_action_delay_floor >= 0) {

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

+26
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
#include "DefaultTimeSyncDelegate.h"
2020
#include "inet/IPAddress.h"
21+
#include <platform/RuntimeOptionsProvider.h>
22+
#include <system/SystemClock.h>
2123

2224
using chip::TimeSyncDataProvider;
2325
using namespace chip::app::Clusters::TimeSynchronization;
@@ -45,3 +47,27 @@ bool DefaultTimeSyncDelegate::IsNTPAddressDomain(chip::CharSpan ntp)
4547
// placeholder implementation
4648
return false;
4749
}
50+
51+
CHIP_ERROR DefaultTimeSyncDelegate::UpdateTimeFromPlatformSource(chip::Callback::Callback<OnTimeSyncCompletion> * callback)
52+
{
53+
System::Clock::Microseconds64 utcTime;
54+
if (chip::app::RuntimeOptionsProvider::Instance().GetSimulateNoInternalTime())
55+
{
56+
return CHIP_ERROR_NOT_IMPLEMENTED;
57+
}
58+
if (System::SystemClock().GetClock_RealTime(utcTime) == CHIP_NO_ERROR)
59+
{
60+
// Default assumes the time came from NTP. Platforms using other sources should overwrite this
61+
// with their own delegates
62+
// Call the callback right away from within this function
63+
callback->mCall(callback->mContext, TimeSourceEnum::kMixedNTP, GranularityEnum::kMillisecondsGranularity);
64+
return CHIP_NO_ERROR;
65+
}
66+
return CHIP_ERROR_NOT_IMPLEMENTED;
67+
}
68+
69+
CHIP_ERROR DefaultTimeSyncDelegate::UpdateTimeUsingNTPFallback(const CharSpan & fallbackNTP,
70+
chip::Callback::Callback<OnFallbackNTPCompletion> * callback)
71+
{
72+
return CHIP_ERROR_NOT_IMPLEMENTED;
73+
}

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

+3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ class DefaultTimeSyncDelegate : public Delegate
3333
bool HandleUpdateDSTOffset(CharSpan name) override;
3434
bool IsNTPAddressValid(CharSpan ntp) override;
3535
bool IsNTPAddressDomain(CharSpan ntp) override;
36+
CHIP_ERROR UpdateTimeFromPlatformSource(chip::Callback::Callback<OnTimeSyncCompletion> * callback) override;
37+
CHIP_ERROR UpdateTimeUsingNTPFallback(const CharSpan & fallbackNTP,
38+
chip::Callback::Callback<OnFallbackNTPCompletion> * callback) override;
3639
};
3740

3841
} // namespace TimeSynchronization

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

+27
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ namespace app {
2929
namespace Clusters {
3030
namespace TimeSynchronization {
3131

32+
typedef void (*OnTimeSyncCompletion)(void * context, TimeSourceEnum timeSource, GranularityEnum granularity);
33+
typedef void (*OnFallbackNTPCompletion)(void * context, bool timeSyncSuccessful);
34+
3235
/** @brief
3336
* Defines methods for implementing application-specific logic for the Time Synchronization Cluster.
3437
*/
@@ -74,6 +77,30 @@ class Delegate
7477
*/
7578
virtual bool IsNTPAddressDomain(const CharSpan ntp) = 0;
7679

80+
/**
81+
* @brief Delegate should attempt to get time from a platform-defined source using the ordering defined in the
82+
* Time source prioritization spec section. Delegate may skip any unsupported sources
83+
* Order: GNSS -> trusted high-resolution external source (PTP, trusted network NTP, cloud) ->
84+
* local network defined NTP (DHCPv6 -> DHCP -> DNS-SD sources)
85+
* If the delegate is unable to support any source, it may return an error immediately. If the delegate is going
86+
* to attempt to obtain time from any source, it returns CHIP_NO_ERROR and calls the callback on completion.
87+
* If the delegate successfully obtains the time, it sets the time using the platform time API (SetClock_RealTime)
88+
* and calls the callback with the time source and granularity set as appropriate.
89+
* If the delegate is unsuccessful in obtaining the time, it calls the callback with timeSource set to kNone and
90+
* granularity set to kNoTimeGranularity.
91+
*/
92+
virtual CHIP_ERROR UpdateTimeFromPlatformSource(chip::Callback::Callback<OnTimeSyncCompletion> * callback) = 0;
93+
94+
/**
95+
* @brief If the delegate supports NTP, it should attempt to update its time using the provided fallbackNTP source.
96+
* If the delegate is successful in obtaining a time from the fallbackNTP, it updates the system time (ex using
97+
* System::SystemClock().SetClock_RealTime) and calls the callback. If the delegate is going to attempt to update
98+
* the time and call the callback, it returns CHIP_NO_ERROR. If the delegate does not support NTP, it may return
99+
* a CHIP_ERROR.
100+
*/
101+
virtual CHIP_ERROR UpdateTimeUsingNTPFallback(const CharSpan & fallbackNTP,
102+
chip::Callback::Callback<OnFallbackNTPCompletion> * callback) = 0;
103+
77104
virtual ~Delegate() = default;
78105

79106
private:

0 commit comments

Comments
 (0)