Detect OpenCommissioningWindow errors in controller apps #10918

pan-apple · 2021-10-25T18:39:24Z

Problem

Fixes TE#6 Mac Chip tool testing: TC-MF-1.24 - Mac Chip tool not giving error response when opening commissioning with 901 seconds timeout #10676
Fixes TE#6 Mac Chip Tool Multiadmin ECM testing: Mac Chip tool doesn't give error response when opening second window without first window timeout #10679

Change overview

The controller API did not have provisions to propagate OpenCommissioningWindow command response status to the controller application. The application was just relying on command send status to determine if the command was successful or not.

This PR adds a callback to the API. The registered callback will convey the command status, and the generated setup payload to the application.

Testing

The failure was reported in TE6 testing while using chip-tool app. Tested this change by running chip-tool with all-cluster-app.

Commission the application
Try opening the commissioning window with 901 seconds timeout. Ensure that chip-tool reports a failure.
Open the commissioning window with 900 seconds timeout. Ensure it succeeds
Try opening the commissioning window while it is already open. Ensure that chip-tool reports a failure.

todo · 2021-10-25T18:39:26Z

- Use cluster enum chip::app::Clusters::AdministratorCommissioning::StatusCode::kBusy

connectedhomeip/src/controller/CHIPDevice.cpp

Lines 327 to 337 in 2c21bb5

    
                   // TODO - Use cluster enum chip::app::Clusters::AdministratorCommissioning::StatusCode::kBusy 
        
                   if (status == 1) 
        
                   { 
        
                       error = CHIP_ERROR_ANOTHER_COMMISSIONING_IN_PROGRESS; 
        
                   } 
        
                   device->mCommissioningWindowCallback->mCall(device->mCommissioningWindowCallback->mContext, device->GetDeviceId(), error, 
        
                                                               SetupPayload()); 
        
               } 
        
           } 
        
           CHIP_ERROR Device::ComputePASEVerifier(uint32_t iterations, uint32_t setupPincode, const ByteSpan & salt,

This comment was generated by todo based on a `TODO` comment in `2c21bb5` in #10918. cc @pan-apple.

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

PR #10918: Size comparison from 66f4ec6 to 2afb9b7

1 build (for telink)

platform	target	config	section	`66f4ec6`	`2afb9b7`
telink	lighting-app	tlsr9518adk80d	bss	69940	69940
			noinit	33216	33216
			text	457672	457672

github-actions · 2021-10-25T22:07:08Z

PR #10918: Size comparison from d7151ac to ec0d926

Increases above 1.0% from d7151ac to ec0d926:

platform	target	config	section	`d7151ac`	`ec0d926`	change	% change
linux	ota-requestor-app	debug	.bss	205696	213376	7680	3.7
	tv-app	debug	.bss	215536	223216	7680	3.6

20 builds (for efr32, k32w, linux, p6, qpg, telink)

platform	target	config	section	`d7151ac`	`ec0d926`	change	% change
efr32	lighting-app	BRD4161A	.bss	113684	113684	0	0.0
			.data	1752	1752	0	0.0
			.text	735816	735812	-4	-0.0
	lock-app	BRD4161A	.bss	111540	111540	0	0.0
			.data	1712	1712	0	0.0
			.text	715080	715080	0	0.0
	window-app	BRD4161A	.bss	111852	111852	0	0.0
			.data	1716	1716	0	0.0
			.text	715876	715876	0	0.0
	lighting-app	BRD4161A+rpc	.bss	130188	130188	0	0.0
			.data	1852	1852	0	0.0
			.text	723232	723236	4	0.0
k32w	lock-app	k32w061+debug	.bss	69196	69196	0	0.0
			.data	1864	1864	0	0.0
			.text	515140	515140	0	0.0
	shell	k32w061+debug	.bss	63256	63256	0	0.0
			.data	672	672	0	0.0
			.text	359572	359572	0	0.0
	lighting-app	k32w061+se05x+release	.bss	78712	78712	0	0.0
			.data	1900	1900	0	0.0
			.text	613776	613680	-96	-0.0
linux	all-clusters-app	debug	.bss	50160	50160	0	0.0
			.data	978	978	0	0.0
			.data.rel.ro	60640	60640	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	1354018	1354018	0	0.0
	chip-tool	debug	.bss	17680	17680	0	0.0
			.data	1584	1584	0	0.0
			.data.rel.ro	94624	94624	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	207396	207492	96	0.0
			.text	3624245	3629061	4816	0.1
	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`	`1023714`	0	0.0
	ota-requestor-app	debug	.bss	205696	213376	7680	3.7
			.data	752	752	0	0.0
			.data.rel.ro	25832	25832	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	128424	0	0.0
			.text	1142146	1147362	5216	0.5
	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	215536	223216	7680	3.6
			.data	2032	2032	0	0.0
			.data.rel.ro	57424	57424	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	`1454994`	656	0.0
	bridge-app	debug+rpc	.bss	51856	51856	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	109740	0	0.0
			.text	`1064293`	`1064293`	0	0.0
	lighting-app	debug+rpc	.bss	41176	41176	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	126897	0	0.0
			.text	1262178	1262178	0	0.0
p6	lock-app	default	.bss	67176	67176	0	0.0
			.data	2416	2416	0	0.0
			.heap	963752	963752	0	0.0
			.text	1126088	1126088	0	0.0
qpg	lighting-app	qpg6100+debug	.bss	52416	52416	0	0.0
			.data	1000	1000	0	0.0
			.text	485044	484940	-104	-0.0
	lock-app	qpg6100+debug	.bss	51360	51360	0	0.0
			.data	956	956	0	0.0
			.text	461264	461264	0	0.0
	persistent-storage-app	qpg6100+debug	.bss	27752	27752	0	0.0
			.data	372	372	0	0.0
			.text	149900	149900	0	0.0
telink	lighting-app	tlsr9518adk80d	bss	69940	69940	0	0.0
			noinit	33216	33216	0	0.0
			text	457672	457672	0	0.0

4 builds (for mbed)

platform	target	config	section	`d7151ac`	`ec0d926`
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

12 builds (for esp32, nrfconnect)

platform	target	config	section	`d7151ac`	`ec0d926`	change
esp32	all-clusters-app	c3devkit	.dram0.bss	58232	58232	0
			.dram0.data	16464	16464	0
			.flash.rodata	198048	198048	0
			.flash.text	874382	874382	0
			.iram0.text	57564	57564	0
		m5stack	.dram0.bss	60736	60736	0
			.dram0.data	32084	32084	0
			.flash.rodata	206808	206808	0
			.flash.text	905451	905539	88
			.iram0.text	125115	125115	0
nrfconnect	lighting-app	nrf52840dk_nrf52840	bss	112136	112136	0
			rodata	97100	97100	0
			text	577204	577204	0
	lock-app	nrf52840dk_nrf52840	bss	111208	111208	0
			rodata	93500	93500	0
			text	558764	558764	0
	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	94700	94700	0
			text	561868	561868	0
	pump-controller-app	nrf52840dk_nrf52840	bss	111204	111204	0
			rodata	93476	93476	0
			text	558508	558508	0
	shell	nrf52840dk_nrf52840	bss	109072	109072	0
			rodata	72536	72536	0
			text	520324	520324	0
	lighting-app	nrf52840dk_nrf52840+rpc	bss	108376	108376	0
			rodata	87876	87876	0
			text	550396	550396	0
		nrf5340dk_nrf5340_cpuapp	bss	113508	113508	0
			rodata	92340	92340	0
			text	506676	506676	0
	lock-app	nrf5340dk_nrf5340_cpuapp	bss	112580	112580	0
			rodata	88760	88760	0
			text	488228	488228	0
	shell	nrf5340dk_nrf5340_cpuapp	bss	110056	110056	0
			rodata	67180	67180	0
			text	440936	440936	0

pan-apple · 2021-10-26T14:44:04Z

/rebase

src/controller/CHIPDevice.cpp

examples/chip-tool/commands/pairing/PairingCommand.cpp

woody-apple · 2021-10-27T07:40:07Z

/rebase

github-actions · 2021-10-27T08:05:51Z

Size increase report for "gn_qpg-example-build" from 4e2eeab

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

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,30301
.debug_loc,0,11870
.debug_str,0,5507
.debug_line,0,4572
.debug_ranges,0,3008
.debug_frame,0,448
.debug_abbrev,0,250
.debug_aranges,0,104
[Unmapped],0,104
.symtab,0,-48
.text,-104,-104

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'

examples/chip-tool/commands/pairing/PairingCommand.cpp

…p#10918) * Detect OpenCommissioningWindow errors in controller apps * fix build * address review comments

boring-cyborg bot added controller examples lib labels Oct 25, 2021

pan-apple force-pushed the ma-issues branch from 2afb9b7 to ec0d926 Compare October 25, 2021 21:29

pullapprove bot requested a review from selissia October 26, 2021 14:44

tcarmelveilleux reviewed Oct 26, 2021

View reviewed changes

src/controller/CHIPDevice.cpp Show resolved Hide resolved

pan-apple force-pushed the ma-issues branch from ec0d926 to 4ebaf1a Compare October 26, 2021 15:58

franck-apple added the TE7 label Oct 26, 2021

franck-apple added this to the Test Event 7 milestone Oct 26, 2021

woody-apple approved these changes Oct 26, 2021

View reviewed changes

bzbarsky-apple reviewed Oct 27, 2021

View reviewed changes

examples/chip-tool/commands/pairing/PairingCommand.cpp Outdated Show resolved Hide resolved

pullapprove bot requested a review from electrocucaracha October 27, 2021 07:40

woody-apple force-pushed the ma-issues branch from 4ebaf1a to a061e78 Compare October 27, 2021 07:40

andy31415 approved these changes Oct 27, 2021

View reviewed changes

examples/chip-tool/commands/pairing/PairingCommand.cpp Outdated Show resolved Hide resolved

pan-apple added 3 commits October 27, 2021 08:32

Detect OpenCommissioningWindow errors in controller apps

60339be

fix build

9c68d1b

address review comments

36ff4f3

pan-apple force-pushed the ma-issues branch from a061e78 to 36ff4f3 Compare October 27, 2021 15:35

jmartinez-silabs approved these changes Oct 27, 2021

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels Oct 27, 2021

pan-apple merged commit 837bd28 into project-chip:master Oct 27, 2021

todo bot mentioned this pull request Oct 27, 2021

- Use cluster enum chip::app::Clusters::AdministratorCommissioning::StatusCode::... #11065

Closed

pan-apple deleted the ma-issues branch October 27, 2021 19:33

PSONALl pushed a commit to PSONALl/connectedhomeip that referenced this pull request Dec 3, 2021

Detect OpenCommissioningWindow errors in controller apps (project-chi…

1e25c66

…p#10918) * Detect OpenCommissioningWindow errors in controller apps * fix build * address review comments

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect OpenCommissioningWindow errors in controller apps #10918

Detect OpenCommissioningWindow errors in controller apps #10918

pan-apple commented Oct 25, 2021

todo bot commented Oct 25, 2021

github-actions bot commented Oct 25, 2021

github-actions bot commented Oct 25, 2021 •

edited

Loading

pan-apple commented Oct 26, 2021

woody-apple commented Oct 27, 2021

github-actions bot commented Oct 27, 2021

Detect OpenCommissioningWindow errors in controller apps #10918

Detect OpenCommissioningWindow errors in controller apps #10918

Conversation

pan-apple commented Oct 25, 2021

Problem

Change overview

Testing

todo bot commented Oct 25, 2021

- Use cluster enum chip::app::Clusters::AdministratorCommissioning::StatusCode::kBusy

This comment was generated by todo based on a TODO comment in 2c21bb5 in #10918. cc @pan-apple.

github-actions bot commented Oct 25, 2021

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

pan-apple commented Oct 26, 2021

woody-apple commented Oct 27, 2021

github-actions bot commented Oct 27, 2021

This comment was generated by todo based on a `TODO` comment in `2c21bb5` in #10918. cc @pan-apple.

github-actions bot commented Oct 25, 2021 •

edited

Loading