Indirect access for System::Clock functions #10056

kpschoedel · 2021-09-29T14:09:50Z

Problem

Historically System::Clock operations were fixed global functions.
Recently PR#9547 converted to a mockable abstract base class, but
transitionally retained the global function API externally.

Change overview

Added SystemClock(), following the model of SystemLayer().
Removed the old global functions.

Testing

CI; no change to functionality expected.
Revised TestSystemClock to use a SetSystemClockForTesting()
function, again following the System::Layer model.

#### Problem Historically `System::Clock` operations were fixed global functions. Recently PR#9547 converted to a mockable abstract base class, but transitionally retained the global function API externally. #### Change overview - Added `SystemClock()`, following the model of `SystemLayer()`. - Removed the old global functions. #### Testing CI. Revised `TestSystemClock` to use a `SetSystemClockForTesting()` function, again following the System::Layer model.

todo · 2021-09-30T14:39:56Z

remove.

connectedhomeip/src/system/SystemClock.h

Lines 115 to 125 in e406848

    
           // DO NOT USE - Temporary backward compatibility functions. TODO: remove. 
        
           inline MonotonicMicroseconds GetMonotonicMicroseconds() 
        
           { 
        
               return SystemClock().GetMonotonicMicroseconds(); 
        
           } 
        
           inline MonotonicMilliseconds GetMonotonicMilliseconds() 
        
           { 
        
               return SystemClock().GetMonotonicMilliseconds(); 
        
           } 
        
           /**

This comment was generated by todo based on a `TODO` comment in `e406848` in #10056. cc @kpschoedel.

github-actions · 2021-09-30T15:10:35Z

Size increase report for "gn_qpg-example-build" from 12a35d8

File	Section	File	VM
chip-qpg6100-lighting-example.out	.text	192	192

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
.debug_info,0,529
.debug_abbrev,0,432
.debug_loc,0,342
.debug_line,0,316
.debug_ranges,0,280
.text,192,192
.symtab,0,80
.strtab,0,18
.shstrtab,0,-2
.debug_str,0,-3
.debug_aranges,0,-32
.debug_frame,0,-44
[Unmapped],0,-192

github-actions · 2021-09-30T15:40:48Z

Size increase report for "nrfconnect-example-build" from 12a35d8

File	Section	File	VM
chip-lock.elf	text	200	200
chip-lock.elf	device_handles	-8	-8
chip-shell.elf	text	232	232
chip-shell.elf	device_handles	-8	-8

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.debug_info,0,531
.debug_abbrev,0,380
.debug_line,0,361
.debug_ranges,0,360
text,200,200
.symtab,0,64
.strtab,0,18
.debug_str,0,3
.shstrtab,0,2
device_handles,-8,-8
.debug_aranges,0,-32
.debug_frame,0,-44
.debug_loc,0,-79

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_line,0,415
.debug_ranges,0,360
.debug_info,0,351
.debug_abbrev,0,314
text,232,232
.debug_loc,0,189
.strtab,0,18
.symtab,0,16
.debug_str,0,3
.shstrtab,0,-2
device_handles,-8,-8
.debug_aranges,0,-40
.debug_frame,0,-64

github-actions · 2021-09-30T15:50:47Z

Size increase report for "esp32-example-build" from 12a35d8

File	Section	File	VM
chip-all-clusters-app.elf	.flash.text	192	192

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_line,0,636
.debug_ranges,0,384
.debug_abbrev,0,383
.flash.text,192,192
.strtab,0,19
.debug_str,0,3
.shstrtab,0,1
.riscv.attributes,0,-3
.debug_aranges,0,-48
.debug_frame,0,-76
.debug_loc,0,-138
[Unmapped],0,-192
.debug_info,0,-2549

kpschoedel · 2021-09-30T18:56:49Z

PR #10056: Size comparison from e406848 to 12a35d8

34 builds

platform	target	config	section	`12a35d8`	`e406848`	change	% change
efr32	lighting-app	BRD4161A	.bss	117956	117956	0	0.0
			.data	1792	1792	0	0.0
			.text	779076	779140	64	0.0
	lock-app	BRD4161A	.bss	115396	115396	0	0.0
			.data	1752	1752	0	0.0
			.text	756416	756496	80	0.0
	window-app	BRD4161A	.bss	117036	117036	0	0.0
			.data	1756	1756	0	0.0
			.text	757820	757884	64	0.0
	lighting-app	BRD4161A+rpc	.bss	131292	131292	0	0.0
			.data	1844	1844	0	0.0
			.text	757984	758064	80	0.0
esp32	all-clusters-app	c3devkit	.dram0.bss	65376	65376	0	0.0
			.dram0.data	16240	16240	0	0.0
			.flash.rodata	200648	200648	0	0.0
			.flash.text	873386	873578	192	0.0
			.iram0.text	57330	57330	0	0.0
		m5stack	.dram0.bss	67880	67880	0	0.0
			.dram0.data	32092	32092	0	0.0
			.flash.rodata	209120	209120	0	0.0
			.flash.text	907451	907547	96	0.0
			.iram0.text	125115	125115	0	0.0
k32w	lock-app	k32w061+debug	.bss	68604	68604	0	0.0
			.data	1856	1856	0	0.0
			.text	513668	513876	208	0.0
	shell	k32w061+debug	.bss	56688	56688	0	0.0
			.data	672	672	0	0.0
			.text	370068	370308	240	0.1
	lighting-app	k32w061+se05x+release	.bss	78536	78536	0	0.0
			.data	1892	1892	0	0.0
			.text	613456	613680	224	0.0
linux	all-clusters-app	debug	.bss	58096	58096	0	0.0
			.data	1010	1010	0	0.0
			.data.rel.ro	58160	58160	0	0.0
			.dynamic	592	592	0	0.0
			.got	4096	4096	0	0.0
			.init	27	27	0	0.0
			.init_array	496	496	0	0.0
			.rodata	133365	133365	0	0.0
			.text	1250482	1250850	368	0.0
	chip-tool	debug	.bss	18864	18864	0	0.0
			.data	1584	1584	0	0.0
			.data.rel.ro	73904	73904	0	0.0
			.dynamic	592	592	0	0.0
			.got	4320	4320	0	0.0
			.init	27	27	0	0.0
			.init_array	392	392	0	0.0
			.rodata	172376	172376	0	0.0
			.text	`2383525`	2383813	288	0.0
	ota-provider-app	debug	.bss	37440	37440	0	0.0
			.data	752	752	0	0.0
			.data.rel.ro	22776	22776	0	0.0
			.dynamic	592	592	0	0.0
			.got	3968	3968	0	0.0
			.init	27	27	0	0.0
			.init_array	416	416	0	0.0
			.rodata	106224	106224	0	0.0
			.text	966978	967314	336	0.0
	ota-requestor-app	debug	.bss	204728	204728	0	0.0
			.data	704	704	0	0.0
			.data.rel.ro	23488	23488	0	0.0
			.dynamic	592	592	0	0.0
			.got	4080	4080	0	0.0
			.init	27	27	0	0.0
			.init_array	464	464	0	0.0
			.rodata	112878	112878	0	0.0
			.text	`1032930`	`1033266`	336	0.0
	shell	debug	.bss	18152	18152	0	0.0
			.data	242	242	0	0.0
			.data.rel.ro	36608	36608	0	0.0
			.dynamic	592	592	0	0.0
			.got	3920	3920	0	0.0
			.init	27	27	0	0.0
			.init_array	336	336	0	0.0
			.rodata	73135	73135	0	0.0
			.text	629346	629746	400	0.1
	tv-app	debug	.bss	218272	218272	0	0.0
			.data	2032	2032	0	0.0
			.data.rel.ro	53960	53960	0	0.0
			.dynamic	592	592	0	0.0
			.got	4368	4368	0	0.0
			.init	27	27	0	0.0
			.init_array	576	576	0	0.0
			.rodata	143326	143326	0	0.0
			.text	1373458	1373794	336	0.0
	bridge-app	debug+rpc	.bss	54384	54384	0	0.0
			.data	976	976	0	0.0
			.data.rel.ro	25096	25096	0	0.0
			.dynamic	592	592	0	0.0
			.got	3952	3952	0	0.0
			.init	27	27	0	0.0
			.init_array	384	384	0	0.0
			.rodata	106760	106760	0	0.0
			.text	1015333	1015669	336	0.0
	lighting-app	debug+rpc	.bss	42936	42936	0	0.0
			.data	1106	1106	0	0.0
			.data.rel.ro	51488	51488	0	0.0
			.dynamic	608	608	0	0.0
			.got	4128	4128	0	0.0
			.init	27	27	0	0.0
			.init_array	520	520	0	0.0
			.rodata	122577	122577	0	0.0
			.text	1199650	`1199986`	336	0.0
mbed	lighting-app	CY8CPROTO_062_4343W+release	.bss	172068	172068	0	0.0
			.data	5368	5368	0	0.0
			.heap	859008	859008	0	0.0
			.text	1150952	`1151144`	192	0.0
	lock-app	CY8CPROTO_062_4343W+release	.bss	170572	170572	0	0.0
			.data	5336	5336	0	0.0
			.heap	860536	860536	0	0.0
			.text	1126808	1127000	192	0.0
nrfconnect	lighting-app	nrf52840dk_nrf52840	bss	112128	112128	0	0.0
			rodata	96880	96880	0	0.0
			text	578952	579148	196	0.0
	lock-app	nrf52840dk_nrf52840	bss	110728	110728	0	0.0
			rodata	92828	92828	0	0.0
			text	559188	559388	200	0.0
	pigweed-app	nrf52840dk_nrf52840	bss	51580	51580	0	0.0
			rodata	45744	45744	0	0.0
			text	338992	338992	0	0.0
	pump-app	nrf52840dk_nrf52840	bss	111152	111152	0	0.0
			rodata	93944	93944	0	0.0
			text	562492	562688	196	0.0
	pump-controller-app	nrf52840dk_nrf52840	bss	110648	110648	0	0.0
			rodata	92652	92652	0	0.0
			text	557968	558140	172	0.0
	shell	nrf52840dk_nrf52840	bss	108756	108756	0	0.0
			rodata	72648	72648	0	0.0
			text	531952	532184	232	0.0
	lighting-app	nrf52840dk_nrf52840+rpc	bss	108364	108364	0	0.0
			rodata	87624	87624	0	0.0
			text	551684	551880	196	0.0
		nrf5340dk_nrf5340_cpuapp	bss	113688	113688	0	0.0
			rodata	92124	92124	0	0.0
			text	508464	508656	192	0.0
	lock-app	nrf5340dk_nrf5340_cpuapp	bss	112288	112288	0	0.0
			rodata	88088	88088	0	0.0
			text	488692	488888	196	0.0
	shell	nrf5340dk_nrf5340_cpuapp	bss	109924	109924	0	0.0
			rodata	67292	67292	0	0.0
			text	452584	452812	228	0.1
p6	lock-app	default	.bss	67856	67856	0	0.0
			.data	2408	2408	0	0.0
			.heap	963080	963080	0	0.0
			.text	1123816	`1123896`	80	0.0
qpg	lighting-app	qpg6100+debug	.bss	53536	53536	0	0.0
			.data	996	996	0	0.0
			.text	485764	485956	192	0.0
	lock-app	qpg6100+debug	.bss	52056	52056	0	0.0
			.data	952	952	0	0.0
			.text	460352	460560	208	0.0
	persistent-storage-app	qpg6100+debug	.bss	17802	17802	0	0.0
			.data	284	284	0	0.0
			.text	102664	102712	48	0.0
telink	lighting-app	tlsr9518adk80d	bss	71084	71084	0	0.0
			noinit	33216	33216	0	0.0
			text	458640	458774	134	0.0

#### Problem Followups from PR project-chip#10056 Indirect access for System::Clock functions Fixes project-chip#10146 Remove GetMonotonicMicroseconds() call Fixes project-chip#10145 remove. Fixes project-chip#10144 The static cast should have the / 1000 within it #### Change overview - Removes the global backward-compatibility wrappers for `GetMonotonic…()`. - Correctly parenthesize the project-chip#10146 cast. #### Testing CI. No changes to functionality intended.

#### Problem Followups from PR #10056 Indirect access for System::Clock functions Fixes #10146 Remove GetMonotonicMicroseconds() call Fixes #10145 remove. Fixes #10144 The static cast should have the / 1000 within it #### Change overview - Removes the global backward-compatibility wrappers for `GetMonotonic…()`. - Correctly parenthesize the #10146 cast. #### Testing CI. No changes to functionality intended.

boring-cyborg bot added app esp32 examples inet lib linux platform labels Sep 29, 2021

kpschoedel added 2 commits September 29, 2021 11:56

Use IsEarlier() in more places with a new AddOffset()

b5d7b61

kpschoedel force-pushed the x7715-clock branch from ec7edaa to b5d7b61 Compare September 29, 2021 16:11

restyled-io bot mentioned this pull request Sep 29, 2021

Restyle Indirect access for System::Clock functions #10059

Closed

woody-apple approved these changes Sep 29, 2021

View reviewed changes

restyle and fix copy-paste

02aad75

kpschoedel mentioned this pull request Sep 29, 2021

Some operations on System::Clock types are not safe #10062

Closed

tcarmelveilleux approved these changes Sep 29, 2021

View reviewed changes

LuDuda approved these changes Sep 29, 2021

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels Sep 29, 2021

msandstedt approved these changes Sep 29, 2021

View reviewed changes

woody-apple added the has conflicts label Sep 30, 2021

emargolis approved these changes Sep 30, 2021

View reviewed changes

kpschoedel added 2 commits September 30, 2021 10:19

Merge remote-tracking branch 'origin/master' into x7715-clock

ff42379

fix crossed paths and add transitional shims

e406848

kpschoedel mentioned this pull request Sep 30, 2021

Generate github comments from size reports (small scale) #10124

Merged

andy31415 mentioned this pull request Oct 1, 2021

The static cast should have the / 1000 within it, rather than outside. Otherwise it's possible that it will overrun after not that long. #10144

Closed

andy31415 merged commit 08fe66d into project-chip:master Oct 1, 2021

todo bot mentioned this pull request Oct 1, 2021

Remove GetMonotonicMicroseconds() call #10146

Closed

kpschoedel mentioned this pull request Oct 1, 2021

Minor System::Clock cleanup #10153

Merged

kpschoedel deleted the x7715-clock branch October 4, 2021 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Indirect access for System::Clock functions #10056

Indirect access for System::Clock functions #10056

kpschoedel commented Sep 29, 2021

todo bot commented Sep 30, 2021

github-actions bot commented Sep 30, 2021

github-actions bot commented Sep 30, 2021

github-actions bot commented Sep 30, 2021

kpschoedel commented Sep 30, 2021

Indirect access for System::Clock functions #10056

Indirect access for System::Clock functions #10056

Conversation

kpschoedel commented Sep 29, 2021

Problem

Change overview

Testing

todo bot commented Sep 30, 2021

remove.

This comment was generated by todo based on a TODO comment in e406848 in #10056. cc @kpschoedel.

github-actions bot commented Sep 30, 2021

github-actions bot commented Sep 30, 2021

github-actions bot commented Sep 30, 2021

kpschoedel commented Sep 30, 2021

This comment was generated by todo based on a `TODO` comment in `e406848` in #10056. cc @kpschoedel.