Add basic codegen and Encode/Decode support for nullable and optional struct/command/event fields #10891

bzbarsky-apple · 2021-10-23T05:51:07Z

Problem

The types and helper code we generate for nullable or optional struct/command/event fields ignores the nullability and optionality. In particular, for a nullable field there is no way to encode it using cluster-objects, and decoding will fail if null is sent.

Change overview

Add a DataModel::Nullable type to represent nullable things.
Add Encode/Decode methods for DataModel::Nullable and chip::Optional (with the latter representing things that are optional).
Fix FabricTable to accept an Optional for the ICAC.
Add support for cluster-objects and YAML codegen for nullable/optional bits. Other codegen pieces are NOT imeplemented, so trying to use these things via the other APIs we have is likely to not work well at all. @vivien-apple @austinh0 I could probably use a hand with updates for the Darwin and Android codegen....

Testing

New test passes. Lots more work to do to make this work in general, though.

github-actions · 2021-10-23T06:14:18Z

PR #10891: Size comparison from 682b650 to 65423ee

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

platform	target	config	section	`682b650`	`65423ee`	change
k32w	lock-app	k32w061+debug	.bss	69228	69228	0
			.data	1864	1864	0
			.text	515076	515124	48
	shell	k32w061+debug	.bss	63256	63256	0
			.data	672	672	0
			.text	359556	359556	0
	lighting-app	k32w061+se05x+release	.bss	78744	78744	0
			.data	1900	1900	0
			.text	613792	613824	32
p6	lock-app	default	.bss	67208	67208	0
			.data	2416	2416	0
			.heap	963720	963720	0
			.text	1125992	1126024	32
qpg	lighting-app	qpg6100+debug	.bss	52456	52456	0
			.data	1004	1004	0
			.text	485048	485080	32
	lock-app	qpg6100+debug	.bss	51400	51400	0
			.data	960	960	0
			.text	461188	461236	48
	persistent-storage-app	qpg6100+debug	.bss	27752	27752	0
			.data	372	372	0
			.text	149896	149896	0
telink	lighting-app	tlsr9518adk80d	bss	69988	69988	0
			noinit	33216	33216	0
			text	457670	457732	62

12 builds (for efr32, linux)

platform	target	config	section	`682b650`	`65423ee`	change	% change
efr32	lighting-app	BRD4161A	.bss	113716	113716	0	0.0
			.data	1752	1752	0	0.0
			.text	735768	735816	48	0.0
	lock-app	BRD4161A	.bss	111572	111572	0	0.0
			.data	1712	1712	0	0.0
			.text	714984	715016	32	0.0
	window-app	BRD4161A	.bss	111884	111884	0	0.0
			.data	1716	1716	0	0.0
			.text	715812	715844	32	0.0
	lighting-app	BRD4161A+rpc	.bss	130220	130220	0	0.0
			.data	1852	1852	0	0.0
			.text	723184	723232	48	0.0
linux	all-clusters-app	debug	.bss	50192	50192	0	0.0
			.data	978	978	0	0.0
			.data.rel.ro	60448	60448	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	134485	134869	384	0.3
			.text	1353266	`1358450`	5184	0.4
	chip-tool	debug	.bss	17712	17712	0	0.0
			.data	1584	1584	0	0.0
			.data.rel.ro	93968	94096	128	0.1
			.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	206884	207708	824	0.4
			.text	3611301	`3627525`	16224	0.4
	ota-provider-app	debug	.bss	37472	37472	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	110584	240	0.2
			.text	1023554	1024658	1104	0.1
	ota-requestor-app	debug	.bss	205728	205728	0	0.0
			.data	752	752	0	0.0
			.data.rel.ro	25800	25800	0	0.0
			.dynamic	592	592	0	0.0
			.got	4144	4144	0	0.0
			.init	27	27	0	0.0
			.init_array	512	512	0	0.0
			.rodata	128424	128664	240	0.2
			.text	`1142274`	`1143554`	1280	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	599506	599778	272	0.0
	tv-app	debug	.bss	215568	215568	0	0.0
			.data	2032	2032	0	0.0
			.data.rel.ro	57408	57408	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	151944	152164	220	0.1
			.text	1454306	`1455346`	1040	0.1
	bridge-app	debug+rpc	.bss	51888	51888	0	0.0
			.data	976	976	0	0.0
			.data.rel.ro	27112	27112	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	109964	224	0.2
			.text	1064085	1065269	1184	0.1
	lighting-app	debug+rpc	.bss	41208	41208	0	0.0
			.data	1106	1106	0	0.0
			.data.rel.ro	53808	53808	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	127121	224	0.2
			.text	`1262034`	`1263218`	1184	0.1

15 builds (for esp32, mbed, nrfconnect)

platform	target	config	section	`682b650`	`65423ee`	change	% change
esp32	all-clusters-app	c3devkit	.dram0.bss	58272	58272	0	0.0
			.dram0.data	16464	16464	0	0.0
			.flash.rodata	197936	197944	8	0.0
			.flash.text	874314	874794	480	0.1
			.iram0.text	57564	57564	0	0.0
		m5stack	.dram0.bss	60776	60776	0	0.0
			.dram0.data	32084	32084	0	0.0
			.flash.rodata	206696	206696	0	0.0
			.flash.text	905471	905831	360	0.0
			.iram0.text	125115	125115	0	0.0
mbed	lighting-app	CY8CPROTO_062_4343W+release	.bss	171092	171092	0	0.0
			.data	5464	5464	0	0.0
			.heap	859888	859888	0	0.0
			.text	`1219112`	`1219112`	0	0.0
	lock-app	CY8CPROTO_062_4343W+release	.bss	170012	170012	0	0.0
			.data	5432	5432	0	0.0
			.heap	861000	861000	0	0.0
			.text	1197072	1197072	0	0.0
	pigweed-app	CY8CPROTO_062_4343W+release	.bss	11760	11760	0	0.0
			.data	4360	4360	0	0.0
			.heap	1020328	1020328	0	0.0
			.text	103064	103064	0	0.0
nrfconnect	lighting-app	nrf52840dk_nrf52840	bss	112168	112168	0	0.0
			rodata	97100	97100	0	0.0
			text	577208	577252	44	0.0
	lock-app	nrf52840dk_nrf52840	bss	111240	111240	0	0.0
			rodata	93500	93500	0	0.0
			text	558696	558740	44	0.0
	pigweed-app	nrf52840dk_nrf52840	bss	51824	51824	0	0.0
			rodata	45776	45776	0	0.0
			text	339456	339456	0	0.0
	pump-app	nrf52840dk_nrf52840	bss	111300	111300	0	0.0
			rodata	94396	94396	0	0.0
			text	561852	561896	44	0.0
	pump-controller-app	nrf52840dk_nrf52840	bss	111236	111236	0	0.0
			rodata	93476	93476	0	0.0
			text	558488	558532	44	0.0
	shell	nrf52840dk_nrf52840	bss	109072	109072	0	0.0
			rodata	72536	72536	0	0.0
			text	520316	520316	0	0.0
	lighting-app	nrf52840dk_nrf52840+rpc	bss	108408	108408	0	0.0
			rodata	87876	87876	0	0.0
			text	550400	550444	44	0.0
		nrf5340dk_nrf5340_cpuapp	bss	113540	113540	0	0.0
			rodata	92340	92340	0	0.0
			text	506680	506724	44	0.0
	lock-app	nrf5340dk_nrf5340_cpuapp	bss	112612	112612	0	0.0
			rodata	88760	88760	0	0.0
			text	488160	488204	44	0.0
	shell	nrf5340dk_nrf5340_cpuapp	bss	110056	110056	0	0.0
			rodata	67180	67180	0	0.0
			text	440924	440924	0	0.0

src/app/zap-templates/zcl/data-model/chip/test-cluster.xml

src/app/data-model/Decode.h

src/app/data-model/Encode.h

src/app/zap-templates/zcl/data-model/chip/test-cluster.xml

Affects command fields, event fields, struct fields.

github-actions · 2021-10-26T20:46:54Z

Size increase report for "gn_qpg-example-build" from 3f7465f

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

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,54291
.debug_str,0,31373
.debug_loc,0,16456
.debug_line,0,8187
.debug_ranges,0,4256
.debug_frame,0,964
.debug_aranges,0,240
.debug_abbrev,0,221
.strtab,0,64
.text,48,48
.symtab,0,32
[Unmapped],0,-48

github-actions · 2021-10-26T21:43:06Z

Size increase report for "esp32-example-build" from 3f7465f

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

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,53026
.debug_str,0,33105
.debug_loc,0,12795
.debug_line,0,10981
.debug_ranges,0,4480
.debug_frame,0,1096
.strtab,0,662
.flash.text,488,488
.debug_aranges,0,248
.debug_abbrev,0,243
.symtab,0,96
.riscv.attributes,0,2
.shstrtab,0,2
[Unmapped],0,-488

github-actions · 2021-10-26T21:43:22Z

Size increase report for "nrfconnect-example-build" from 3f7465f

File	Section	File	VM
chip-lock.elf	text	44	44
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,52600
.debug_str,0,31371
.debug_loc,0,15333
.debug_line,0,8139
.debug_ranges,0,3976
.debug_frame,0,1000
.debug_aranges,0,256
.debug_abbrev,0,181
.strtab,0,161
.symtab,0,64
text,44,44
device_handles,4,4
.shstrtab,0,-1

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

sections,vmsize,filesize
.debug_line,0,16
.debug_loc,0,-16

woody-apple · 2021-10-27T01:49:41Z

Fast tracking, has approvals and is past the timeframe required

… struct/command/event fields (project-chip#10891) * Add basic support for nullable and optional fields. Affects command fields, event fields, struct fields. * Add some unit tests for nullable and optional encode/decode * Add simple optional/nullable end-to-end test.

boring-cyborg bot added app controller darwin examples lib transport labels Oct 23, 2021

bzbarsky-apple mentioned this pull request Oct 23, 2021

Flag optional fields in the OTA clusters as optional in the xml #10892

Closed

bzbarsky-apple requested review from vivien-apple and mrjerryjohns October 23, 2021 05:51

pullapprove bot requested review from sagar-apple, saurabhst, tcarmelveilleux, tecimovic, wbschiller, woody-apple and yunhanw-google October 23, 2021 05:52

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

woody-apple approved these changes Oct 23, 2021

View reviewed changes

mrjerryjohns reviewed Oct 25, 2021

View reviewed changes

src/app/zap-templates/zcl/data-model/chip/test-cluster.xml Outdated Show resolved Hide resolved

mrjerryjohns suggested changes Oct 25, 2021

View reviewed changes

src/app/data-model/Decode.h Outdated Show resolved Hide resolved

src/app/data-model/Encode.h Show resolved Hide resolved

src/app/zap-templates/zcl/data-model/chip/test-cluster.xml Show resolved Hide resolved

src/app/zap-templates/zcl/data-model/chip/test-cluster.xml Show resolved Hide resolved

bzbarsky-apple added 3 commits October 26, 2021 16:13

Add basic support for nullable and optional fields.

853edca

Affects command fields, event fields, struct fields.

Add some unit tests for nullable and optional encode/decode

87d2252

Add simple optional/nullable end-to-end test.

e9612f5

bzbarsky-apple force-pushed the nullable-optional branch from 65423ee to e9612f5 Compare October 26, 2021 20:16

pullapprove bot requested a review from selissia October 26, 2021 20:17

bzbarsky-apple mentioned this pull request Oct 26, 2021

Add TestCluster example(s) of nullable attribute(s) #10993

Closed

bzbarsky-apple requested a review from mrjerryjohns October 26, 2021 20:18

yunhanw-google approved these changes Oct 26, 2021

View reviewed changes

mrjerryjohns approved these changes Oct 26, 2021

View reviewed changes

woody-apple added the fast track label Oct 27, 2021

woody-apple merged commit 3143c27 into project-chip:master Oct 27, 2021

bzbarsky-apple deleted the nullable-optional branch October 27, 2021 02:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic codegen and Encode/Decode support for nullable and optional struct/command/event fields #10891

Add basic codegen and Encode/Decode support for nullable and optional struct/command/event fields #10891

bzbarsky-apple commented Oct 23, 2021

github-actions bot commented Oct 23, 2021 •

edited

Loading

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

woody-apple commented Oct 27, 2021

Add basic codegen and Encode/Decode support for nullable and optional struct/command/event fields #10891

Add basic codegen and Encode/Decode support for nullable and optional struct/command/event fields #10891

Conversation

bzbarsky-apple commented Oct 23, 2021

Problem

Change overview

Testing

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

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

woody-apple commented Oct 27, 2021

github-actions bot commented Oct 23, 2021 •

edited

Loading