Move basic types from app to core #10925

mlepage-google · 2021-10-25T19:36:40Z

We have a lot of low level types (e.g. FabricIndex) defined in
app/util/basic-types.h. These need to be used by other modules outside
of app (e.g. messaging, access control) which are lower level than app.

Right now the best low level place for these types is in lib/core. So,
moving them there for now, in a header called DataModelTypes.h. If we
need a more intermediate level (above core, below the other modules) in
the future, we can refactor then.

Issue #10924

We have a lot of low level types (e.g. FabricIndex) defined in app/util/basic-types.h. These need to be used by other modules outside of app (e.g. messaging, access control) which are lower level than app. Right now the best low level place for these types is in lib/core. So, moving them there for now, in a header called DataModelTypes.h. If we need a more intermediate level (above core, below the other modules) in the future, we can refactor then. Issue project-chip#10924

github-actions · 2021-10-25T19:55:23Z

PR #10925: Size comparison from b3c4164 to 17acb32

1 build (for telink)

platform	target	config	section	`b3c4164`	`17acb32`
telink	lighting-app	tlsr9518adk80d	bss	69940	69940
			noinit	33216	33216
			text	457672	457672

Basic types (FabricIndex etc.) were moved in PR project-chip#10925 from app to lib/core, so now they can be used from this module.

src/lib/core/DataModelTypes.h

github-actions · 2021-10-25T21:11:58Z

PR #10925: Size comparison from 66f4ec6 to 59ec487

24 builds (for efr32, k32w, linux, mbed, p6, qpg, telink)

platform	target	config	section	`66f4ec6`	`59ec487`
efr32	lighting-app	BRD4161A	.bss	113684	113684
			.data	1752	1752
			.text	735768	735768
	lock-app	BRD4161A	.bss	111540	111540
			.data	1712	1712
			.text	715032	715032
	window-app	BRD4161A	.bss	111852	111852
			.data	1716	1716
			.text	715828	715828
	lighting-app	BRD4161A+rpc	.bss	130188	130188
			.data	1852	1852
			.text	723184	723184
k32w	lock-app	k32w061+debug	.bss	69196	69196
			.data	1864	1864
			.text	515140	515140
	shell	k32w061+debug	.bss	63256	63256
			.data	672	672
			.text	359572	359572
	lighting-app	k32w061+se05x+release	.bss	78712	78712
			.data	1900	1900
			.text	613776	613776
linux	all-clusters-app	debug	.bss	50160	50160
			.data	978	978
			.data.rel.ro	60640	60640
			.dynamic	592	592
			.got	4088	4088
			.init	27	27
			.init_array	512	512
			.rodata	134517	134517
			.text	1353986	1353986
	chip-tool	debug	.bss	17680	17680
			.data	1584	1584
			.data.rel.ro	94624	94624
			.dynamic	592	592
			.got	4368	4368
			.init	27	27
			.init_array	416	416
			.rodata	207396	207396
			.text	`3624229`	`3624229`
	ota-provider-app	debug	.bss	37440	37440
			.data	752	752
			.data.rel.ro	24488	24488
			.dynamic	592	592
			.got	4016	4016
			.init	27	27
			.init_array	440	440
			.rodata	110344	110344
			.text	1023698	1023698
	ota-requestor-app	debug	.bss	205696	205696
			.data	752	752
			.data.rel.ro	25832	25832
			.dynamic	592	592
			.got	4144	4144
			.init	27	27
			.init_array	512	512
			.rodata	128424	128424
			.text	`1142130`	`1142130`
	shell	debug	.bss	16136	16136
			.data	242	242
			.data.rel.ro	36496	36496
			.dynamic	592	592
			.got	3528	3528
			.init	27	27
			.init_array	336	336
			.rodata	76495	76495
			.text	599602	599602
	tv-app	debug	.bss	215536	215536
			.data	2032	2032
			.data.rel.ro	57424	57424
			.dynamic	592	592
			.got	4408	4408
			.init	27	27
			.init_array	608	608
			.rodata	152072	152072
			.text	1454322	1454322
	bridge-app	debug+rpc	.bss	51856	51856
			.data	976	976
			.data.rel.ro	27112	27112
			.dynamic	592	592
			.got	3952	3952
			.init	27	27
			.init_array	400	400
			.rodata	109740	109740
			.text	`1064261`	`1064261`
	lighting-app	debug+rpc	.bss	41176	41176
			.data	1106	1106
			.data.rel.ro	53808	53808
			.dynamic	608	608
			.got	4112	4112
			.init	27	27
			.init_array	528	528
			.rodata	126897	126897
			.text	1262162	1262162
mbed	all-clusters-app	CY8CPROTO_062_4343W+release	.bss	179596	179596
			.data	5192	5192
			.heap	851656	851656
			.text	`1251216`	`1251216`
	lighting-app	CY8CPROTO_062_4343W+release	.bss	171060	171060
			.data	5464	5464
			.heap	859920	859920
			.text	1219256	1219256
	lock-app	CY8CPROTO_062_4343W+release	.bss	169980	169980
			.data	5432	5432
			.heap	861032	861032
			.text	1197280	1197280
	pigweed-app	CY8CPROTO_062_4343W+release	.bss	11760	11760
			.data	4360	4360
			.heap	1020328	1020328
			.text	103064	103064
p6	lock-app	default	.bss	67176	67176
			.data	2416	2416
			.heap	963752	963752
			.text	1126040	1126040
qpg	lighting-app	qpg6100+debug	.bss	52416	52416
			.data	1000	1000
			.text	485044	485044
	lock-app	qpg6100+debug	.bss	51360	51360
			.data	956	956
			.text	461264	461264
	persistent-storage-app	qpg6100+debug	.bss	27752	27752
			.data	372	372
			.text	149900	149900
telink	lighting-app	tlsr9518adk80d	bss	69940	69940
			noinit	33216	33216
			text	457672	457672

12 builds (for esp32, nrfconnect)

platform	target	config	section	`66f4ec6`	`59ec487`
esp32	all-clusters-app	c3devkit	.dram0.bss	58232	58232
			.dram0.data	16464	16464
			.flash.rodata	198048	198048
			.flash.text	874382	874382
			.iram0.text	57564	57564
		m5stack	.dram0.bss	60736	60736
			.dram0.data	32084	32084
			.flash.rodata	206808	206808
			.flash.text	905539	905539
			.iram0.text	125115	125115
nrfconnect	lighting-app	nrf52840dk_nrf52840	bss	112136	112136
			rodata	97100	97100
			text	577204	577204
	lock-app	nrf52840dk_nrf52840	bss	111208	111208
			rodata	93500	93500
			text	558764	558764
	pigweed-app	nrf52840dk_nrf52840	bss	51824	51824
			rodata	45776	45776
			text	339456	339456
	pump-app	nrf52840dk_nrf52840	bss	111308	111308
			rodata	94700	94700
			text	561868	561868
	pump-controller-app	nrf52840dk_nrf52840	bss	111204	111204
			rodata	93476	93476
			text	558508	558508
	shell	nrf52840dk_nrf52840	bss	109072	109072
			rodata	72536	72536
			text	520324	520324
	lighting-app	nrf52840dk_nrf52840+rpc	bss	108376	108376
			rodata	87876	87876
			text	550396	550396
		nrf5340dk_nrf5340_cpuapp	bss	113508	113508
			rodata	92340	92340
			text	506676	506676
	lock-app	nrf5340dk_nrf5340_cpuapp	bss	112580	112580
			rodata	88760	88760
			text	488228	488228
	shell	nrf5340dk_nrf5340_cpuapp	bss	110056	110056
			rodata	67180	67180
			text	440936	440936

andy31415 · 2021-10-26T14:57:32Z

fast track rationale: trivial moving of data for better centralized access.

github-actions · 2021-10-26T15:22:13Z

PR #10925: Size comparison from 596d838 to d7fd084

8 builds (for k32w, p6, qpg, telink)

platform	target	config	section	`596d838`	`d7fd084`	change
k32w	lock-app	k32w061+debug	.bss	69196	69196	0
			.data	1864	1864	0
			.text	515240	515336	96
	shell	k32w061+debug	.bss	63256	63256	0
			.data	672	672	0
			.text	359572	359572	0
	lighting-app	k32w061+se05x+release	.bss	78720	78720	0
			.data	1900	1900	0
			.text	613876	613972	96
p6	lock-app	default	.bss	67184	67184	0
			.data	2416	2416	0
			.heap	963744	963744	0
			.text	1126192	1126288	96
qpg	lighting-app	qpg6100+debug	.bss	52416	52416	0
			.data	1000	1000	0
			.text	485144	485240	96
	lock-app	qpg6100+debug	.bss	51368	51368	0
			.data	956	956	0
			.text	461364	461460	96
	persistent-storage-app	qpg6100+debug	.bss	27752	27752	0
			.data	372	372	0
			.text	149900	149900	0
telink	lighting-app	tlsr9518adk80d	bss	69944	69944	0
			noinit	33216	33216	0
			text	457672	457772	100

12 builds (for efr32, linux)

platform	target	config	section	`596d838`	`d7fd084`	change	% change
efr32	lighting-app	BRD4161A	.bss	113692	113692	0	0.0
			.data	1752	1752	0	0.0
			.text	735912	736008	96	0.0
	lock-app	BRD4161A	.bss	111548	111548	0	0.0
			.data	1712	1712	0	0.0
			.text	715176	715272	96	0.0
	window-app	BRD4161A	.bss	111860	111860	0	0.0
			.data	1716	1716	0	0.0
			.text	716076	716172	96	0.0
	lighting-app	BRD4161A+rpc	.bss	130196	130196	0	0.0
			.data	1852	1852	0	0.0
			.text	723336	723432	96	0.0
linux	all-clusters-app	debug	.bss	50160	50160	0	0.0
			.data	978	978	0	0.0
			.data.rel.ro	60800	60800	0	0.0
			.dynamic	592	592	0	0.0
			.got	4088	4088	0	0.0
			.init	27	27	0	0.0
			.init_array	512	512	0	0.0
			.rodata	134517	134517	0	0.0
			.text	1356946	1357522	576	0.0
	chip-tool	debug	.bss	17680	17680	0	0.0
			.data	1584	1584	0	0.0
			.data.rel.ro	94896	94896	0	0.0
			.dynamic	592	592	0	0.0
			.got	4368	4368	0	0.0
			.init	27	27	0	0.0
			.init_array	416	416	0	0.0
			.rodata	208068	208068	0	0.0
			.text	3642741	3653045	10304	0.3
	ota-provider-app	debug	.bss	37440	37440	0	0.0
			.data	752	752	0	0.0
			.data.rel.ro	24488	24488	0	0.0
			.dynamic	592	592	0	0.0
			.got	4016	4016	0	0.0
			.init	27	27	0	0.0
			.init_array	440	440	0	0.0
			.rodata	110344	110344	0	0.0
			.text	`1023714`	1024306	592	0.1
	ota-requestor-app	debug	.bss	208320	208320	0	0.0
			.data	752	752	0	0.0
			.data.rel.ro	25928	25928	0	0.0
			.dynamic	592	592	0	0.0
			.got	4144	4144	0	0.0
			.init	27	27	0	0.0
			.init_array	520	520	0	0.0
			.rodata	129000	129000	0	0.0
			.text	1145602	1146258	656	0.1
	shell	debug	.bss	16136	16136	0	0.0
			.data	242	242	0	0.0
			.data.rel.ro	36496	36496	0	0.0
			.dynamic	592	592	0	0.0
			.got	3528	3528	0	0.0
			.init	27	27	0	0.0
			.init_array	336	336	0	0.0
			.rodata	76495	76495	0	0.0
			.text	599458	599458	0	0.0
	tv-app	debug	.bss	215568	215568	0	0.0
			.data	2032	2032	0	0.0
			.data.rel.ro	58192	58192	0	0.0
			.dynamic	592	592	0	0.0
			.got	4408	4408	0	0.0
			.init	27	27	0	0.0
			.init_array	608	608	0	0.0
			.rodata	152072	152072	0	0.0
			.text	1454338	1455010	672	0.0
	bridge-app	debug+rpc	.bss	51856	51856	0	0.0
			.data	976	976	0	0.0
			.data.rel.ro	27272	27272	0	0.0
			.dynamic	592	592	0	0.0
			.got	3952	3952	0	0.0
			.init	27	27	0	0.0
			.init_array	400	400	0	0.0
			.rodata	109740	109740	0	0.0
			.text	`1064293`	`1064885`	592	0.1
	lighting-app	debug+rpc	.bss	41176	41176	0	0.0
			.data	1106	1106	0	0.0
			.data.rel.ro	53968	53968	0	0.0
			.dynamic	608	608	0	0.0
			.got	4112	4112	0	0.0
			.init	27	27	0	0.0
			.init_array	528	528	0	0.0
			.rodata	126897	126897	0	0.0
			.text	1262178	`1262770`	592	0.0

4 builds (for mbed)

platform	target	config	section	`596d838`	`d7fd084`	change
mbed	all-clusters-app	CY8CPROTO_062_4343W+release	.bss	179604	179604	0
			.data	5192	5192	0
			.heap	851648	851648	0
			.text	`1251648`	`1251712`	64
	lighting-app	CY8CPROTO_062_4343W+release	.bss	171060	171060	0
			.data	5464	5464	0
			.heap	859920	859920	0
			.text	1219352	1219416	64
	lock-app	CY8CPROTO_062_4343W+release	.bss	169988	169988	0
			.data	5432	5432	0
			.heap	861024	861024	0
			.text	1197384	`1197448`	64
	pigweed-app	CY8CPROTO_062_4343W+release	.bss	11760	11760	0
			.data	4360	4360	0
			.heap	1020328	1020328	0
			.text	103064	103064	0

12 builds (for esp32, nrfconnect)

platform	target	config	section	`596d838`	`d7fd084`	change
esp32	all-clusters-app	c3devkit	.dram0.bss	58232	58232	0
			.dram0.data	16464	16464	0
			.flash.rodata	198176	198176	0
			.flash.text	874742	874842	100
			.iram0.text	57564	57564	0
		m5stack	.dram0.bss	60744	60744	0
			.dram0.data	32084	32084	0
			.flash.rodata	206928	206928	0
			.flash.text	905807	905919	112
			.iram0.text	125115	125115	0
nrfconnect	lighting-app	nrf52840dk_nrf52840	bss	112136	112136	0
			rodata	97200	97200	0
			text	577204	577296	92
	lock-app	nrf52840dk_nrf52840	bss	111208	111208	0
			rodata	93600	93600	0
			text	558764	558856	92
	pigweed-app	nrf52840dk_nrf52840	bss	51824	51824	0
			rodata	45776	45776	0
			text	339456	339456	0
	pump-app	nrf52840dk_nrf52840	bss	111308	111308	0
			rodata	94800	94800	0
			text	561868	561960	92
	pump-controller-app	nrf52840dk_nrf52840	bss	111208	111208	0
			rodata	93576	93576	0
			text	558508	558600	92
	shell	nrf52840dk_nrf52840	bss	109072	109072	0
			rodata	72536	72536	0
			text	520324	520324	0
	lighting-app	nrf52840dk_nrf52840+rpc	bss	108380	108380	0
			rodata	87976	87976	0
			text	550396	550488	92
		nrf5340dk_nrf5340_cpuapp	bss	113512	113512	0
			rodata	92440	92440	0
			text	506676	506768	92
	lock-app	nrf5340dk_nrf5340_cpuapp	bss	112584	112584	0
			rodata	88860	88860	0
			text	488228	488320	92
	shell	nrf5340dk_nrf5340_cpuapp	bss	110056	110056	0
			rodata	67180	67180	0
			text	440936	440936	0

github-actions · 2021-10-26T15:34:41Z

Size increase report for "gn_qpg-example-build" from 596d838

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

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,1270
.debug_loc,0,858
.debug_str,0,351
.strtab,0,308
.debug_ranges,0,128
.symtab,0,96
.text,96,96
.debug_frame,0,76
.debug_abbrev,0,24
.debug_aranges,0,24
[Unmapped],0,-96
.debug_line,0,-4551

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'

github-actions · 2021-10-26T16:19:48Z

Size increase report for "esp32-example-build" from 596d838

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

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_info,0,709
.debug_loc,0,611
.debug_str,0,353
.strtab,0,308
.debug_ranges,0,104
.flash.text,100,100
.debug_frame,0,88
.symtab,0,48
.debug_abbrev,0,24
.debug_aranges,0,24
.riscv.attributes,0,-2
[Unmapped],0,-100
.debug_line,0,-4235

github-actions · 2021-10-26T16:20:00Z

Size increase report for "nrfconnect-example-build" from 596d838

File	Section	File	VM
chip-lock.elf	text	92	92
chip-lock.elf	device_handles	4	4

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,1276
.debug_loc,0,879
.debug_str,0,353
.strtab,0,308
.debug_ranges,0,128
.symtab,0,96
text,92,92
.debug_frame,0,76
.debug_abbrev,0,24
.debug_aranges,0,24
device_handles,4,4
.debug_line,0,-2120

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

sections,vmsize,filesize
.debug_line,0,-1256

Basic types (FabricIndex etc.) were moved in PR project-chip#10925 from app to lib/core, so now they can be used from this module.

* Add initial prototype of AccessControl module - Not complete, always allows actions - Not hooked up to interaction model or messaging layer - Progress toward issues #10236 and #10249 - Fully isolated as a module - Has unit tests * Remove file comments from files * Add 'k' prefix to enum values * Restyled by whitespace * Restyled by clang-format * Restyled by gn * Remove "empty" .cpp files * Apply suggestions from code review * Apply suggestions from code review * Fix compatibility under different compilers * Fix unit test compatability on different compilers * Restyled by clang-format * Change forward declaration to include Allows tooling to detect circular dependencies. * Changes from code review suggestions - rename namespace access --> Access - rename DataProvider --> AccessControlDataProvider - decouple DataProvider lifecycle (Init/Finish) - rename DataProviderImpl --> ExampleAccessControlDataProvider - change GetInstance/SetInstance to global functions - remove Config.h since global instance must be set - change EntryIterator::Next to return pointer - add comments to Privilege and AuthMode - remove SubjectDescriptor.isCommissioning for now - improve naming of CAT subjects in SubjectDescriptor - change SubjectId typedef to use NodeId * Make tests table-driven Should also fix some build complaints on ESP32 * Restyled by clang-format * Restyle * Add more metatesting Ensure not just that results are correct, but that they were correctly obtained. * Restyled by clang-format * Change enums to enum classes * Address review comments * Use basic types in lib/core Basic types (FabricIndex etc.) were moved in PR #10925 from app to lib/core, so now they can be used from this module. * A bit of cleanup * Refactor SubjectId and SubjectDescriptor Also, remove CatId and move PasscodeId into lib/core. * Restyled by clang-format * Add clarifying examples to documentation. Co-authored-by: Restyled.io <[email protected]>

We have a lot of low level types (e.g. FabricIndex) defined in app/util/basic-types.h. These need to be used by other modules outside of app (e.g. messaging, access control) which are lower level than app. Right now the best low level place for these types is in lib/core. So, moving them there for now, in a header called DataModelTypes.h. If we need a more intermediate level (above core, below the other modules) in the future, we can refactor then. Issue project-chip#10924

* Add initial prototype of AccessControl module - Not complete, always allows actions - Not hooked up to interaction model or messaging layer - Progress toward issues project-chip#10236 and project-chip#10249 - Fully isolated as a module - Has unit tests * Remove file comments from files * Add 'k' prefix to enum values * Restyled by whitespace * Restyled by clang-format * Restyled by gn * Remove "empty" .cpp files * Apply suggestions from code review * Apply suggestions from code review * Fix compatibility under different compilers * Fix unit test compatability on different compilers * Restyled by clang-format * Change forward declaration to include Allows tooling to detect circular dependencies. * Changes from code review suggestions - rename namespace access --> Access - rename DataProvider --> AccessControlDataProvider - decouple DataProvider lifecycle (Init/Finish) - rename DataProviderImpl --> ExampleAccessControlDataProvider - change GetInstance/SetInstance to global functions - remove Config.h since global instance must be set - change EntryIterator::Next to return pointer - add comments to Privilege and AuthMode - remove SubjectDescriptor.isCommissioning for now - improve naming of CAT subjects in SubjectDescriptor - change SubjectId typedef to use NodeId * Make tests table-driven Should also fix some build complaints on ESP32 * Restyled by clang-format * Restyle * Add more metatesting Ensure not just that results are correct, but that they were correctly obtained. * Restyled by clang-format * Change enums to enum classes * Address review comments * Use basic types in lib/core Basic types (FabricIndex etc.) were moved in PR project-chip#10925 from app to lib/core, so now they can be used from this module. * A bit of cleanup * Refactor SubjectId and SubjectDescriptor Also, remove CatId and move PasscodeId into lib/core. * Restyled by clang-format * Add clarifying examples to documentation. Co-authored-by: Restyled.io <[email protected]>

boring-cyborg bot added app lib labels Oct 25, 2021

pullapprove bot requested review from vivien-apple, wbschiller, woody-apple and yunhanw-google October 25, 2021 19:36

pullapprove bot added the review - pending label Oct 25, 2021

mlepage-google assigned andy31415 Oct 25, 2021

Merge branch 'master' into move-basic-types-header-lower

59ec487

mlepage-google added a commit to mlepage-google/connectedhomeip that referenced this pull request Oct 25, 2021

Use basic types in lib/core

b4008af

Basic types (FabricIndex etc.) were moved in PR project-chip#10925 from app to lib/core, so now they can be used from this module.

mlepage-google mentioned this pull request Oct 25, 2021

Add initial prototype of AccessControl module #10579

Merged

yunhanw-google reviewed Oct 25, 2021

View reviewed changes

src/lib/core/DataModelTypes.h Show resolved Hide resolved

pullapprove bot requested a review from selissia October 26, 2021 13:42

Merge branch 'master' into move-basic-types-header-lower

d7fd084

andy31415 approved these changes Oct 26, 2021

View reviewed changes

andy31415 added the fast track label Oct 26, 2021

andy31415 merged commit fd34f21 into project-chip:master Oct 26, 2021

woody-apple pushed a commit to mlepage-google/connectedhomeip that referenced this pull request Oct 27, 2021

Use basic types in lib/core

0f9986e

Basic types (FabricIndex etc.) were moved in PR project-chip#10925 from app to lib/core, so now they can be used from this module.

mlepage-google deleted the move-basic-types-header-lower branch February 15, 2022 15:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move basic types from app to core #10925

Move basic types from app to core #10925

mlepage-google commented Oct 25, 2021

github-actions bot commented Oct 25, 2021

github-actions bot commented Oct 25, 2021 •

edited

Loading

andy31415 commented Oct 26, 2021

github-actions bot commented Oct 26, 2021 •

edited

Loading

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

Move basic types from app to core #10925

Move basic types from app to core #10925

Conversation

mlepage-google commented Oct 25, 2021

github-actions bot commented Oct 25, 2021

github-actions bot commented Oct 25, 2021 • edited Loading

andy31415 commented Oct 26, 2021

github-actions bot commented Oct 26, 2021 • edited Loading

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 25, 2021 •

edited

Loading

github-actions bot commented Oct 26, 2021 •

edited

Loading