Remove usage of GetDevice() API in Android controller #10612

pan-apple · 2021-10-18T14:56:26Z

Problem

Android Controller code needs update to remove usage of GetDevice() API.

Change overview

Use GetPeerAddressAndPort() to get device's address
Use ReleaseDeviceById() to disconnect the device.
Remaining usage of GetDevice() have been removed in [Android] Make openPairingWindow accept a device pointer #10590

Testing

Run existing CI tests to ensure that the controller behavior is intact.

src/controller/java/CHIPDeviceController-JNI.cpp

github-actions · 2021-10-18T15:26:31Z

PR #10612: Size comparison from dc93d27 to 92a703c

12 builds

platform	target	config	section	`dc93d27`	`92a703c`
efr32	lighting-app	BRD4161A	.bss	118036	118036
			.data	1800	1800
			.text	782724	782724
	lock-app	BRD4161A	.bss	115892	115892
			.data	1760	1760
			.text	761868	761868
	window-app	BRD4161A	.bss	116212	116212
			.data	1764	1764
			.text	762792	762792
	lighting-app	BRD4161A+rpc	.bss	131364	131364
			.data	1852	1852
			.text	762468	762468
k32w	lock-app	k32w061+debug	.bss	69052	69052
			.data	1864	1864
			.text	515320	515320
	shell	k32w061+debug	.bss	55080	55080
			.data	672	672
			.text	357300	357300
	lighting-app	k32w061+se05x+release	.bss	78576	78576
			.data	1900	1900
			.text	614088	614088
p6	lock-app	default	.bss	68216	68216
			.data	2416	2416
			.heap	962712	962712
			.text	1126424	1126424
qpg	lighting-app	qpg6100+debug	.bss	53552	53552
			.data	996	996
			.text	486400	486400
	lock-app	qpg6100+debug	.bss	52488	52488
			.data	952	952
			.text	462496	462496
	persistent-storage-app	qpg6100+debug	.bss	17778	17778
			.data	280	280
			.text	102704	102704
telink	lighting-app	tlsr9518adk80d	bss	71000	71000
			noinit	33216	33216
			text	458218	458218

10 builds

platform	target	config	section	`dc93d27`	`92a703c`
linux	all-clusters-app	debug	.bss	52176	52176
			.data	978	978
			.data.rel.ro	58688	58688
			.dynamic	592	592
			.got	4072	4072
			.init	27	27
			.init_array	512	512
			.rodata	135957	135957
			.text	1326946	1326946
	chip-tool	debug	.bss	17552	17552
			.data	1584	1584
			.data.rel.ro	84896	84896
			.dynamic	592	592
			.got	4336	4336
			.init	27	27
			.init_array	416	416
			.rodata	176928	176928
			.text	2458165	2458165
	ota-provider-app	debug	.bss	37472	37472
			.data	752	752
			.data.rel.ro	23176	23176
			.dynamic	592	592
			.got	4008	4008
			.init	27	27
			.init_array	440	440
			.rodata	109832	109832
			.text	1010210	1010210
	ota-requestor-app	debug	.bss	205728	205728
			.data	752	752
			.data.rel.ro	24488	24488
			.dynamic	592	592
			.got	4136	4136
			.init	27	27
			.init_array	512	512
			.rodata	128328	128328
			.text	1129522	1129522
	shell	debug	.bss	16072	16072
			.data	242	242
			.data.rel.ro	35120	35120
			.dynamic	592	592
			.got	3496	3496
			.init	27	27
			.init_array	336	336
			.rodata	71855	71855
			.text	569746	569746
	tv-app	debug	.bss	216400	216400
			.data	2032	2032
			.data.rel.ro	55872	55872
			.dynamic	592	592
			.got	4400	4400
			.init	27	27
			.init_array	608	608
			.rodata	151848	151848
			.text	`1463698`	`1463698`
	bridge-app	debug+rpc	.bss	52912	52912
			.data	976	976
			.data.rel.ro	25784	25784
			.dynamic	592	592
			.got	3944	3944
			.init	27	27
			.init_array	400	400
			.rodata	110452	110452
			.text	1051717	1051717
	lighting-app	debug+rpc	.bss	42232	42232
			.data	1106	1106
			.data.rel.ro	52496	52496
			.dynamic	608	608
			.got	4104	4104
			.init	27	27
			.init_array	528	528
			.rodata	127665	127665
			.text	`1253122`	`1253122`
mbed	lighting-app	CY8CPROTO_062_4343W+release	.bss	172164	172164
			.data	5464	5464
			.heap	858816	858816
			.text	`1219736`	`1219736`
	lock-app	CY8CPROTO_062_4343W+release	.bss	171084	171084
			.data	5432	5432
			.heap	859928	859928
			.text	1197632	1197632

2 builds

platform	target	config	section	`dc93d27`	`92a703c`	change
esp32	all-clusters-app	c3devkit	.dram0.bss	60296	60296	0
			.dram0.data	16192	16192	0
			.flash.rodata	198784	198784	0
			.flash.text	870186	870186	0
			.iram0.text	57330	57330	0
		m5stack	.dram0.bss	62792	62792	0
			.dram0.data	32084	32084	0
			.flash.rodata	207216	207216	0
			.flash.text	900547	900639	92
			.iram0.text	125115	125115	0

…nter()

pan-apple · 2021-10-18T15:59:47Z

@austinh0 , could you also please review this PR, as this is updating Android controller code.

github-actions · 2021-10-18T16:26:21Z

PR #10612: Size comparison from 9b0afc4 to cbe8747

8 builds

platform	target	config	section	`9b0afc4`	`cbe8747`
k32w	lock-app	k32w061+debug	.bss	69052	69052
			.data	1864	1864
			.text	515320	515320
	shell	k32w061+debug	.bss	55080	55080
			.data	672	672
			.text	357300	357300
	lighting-app	k32w061+se05x+release	.bss	78576	78576
			.data	1900	1900
			.text	614088	614088
p6	lock-app	default	.bss	68216	68216
			.data	2416	2416
			.heap	962712	962712
			.text	1126424	1126424
qpg	lighting-app	qpg6100+debug	.bss	53552	53552
			.data	996	996
			.text	486400	486400
	lock-app	qpg6100+debug	.bss	52488	52488
			.data	952	952
			.text	462496	462496
	persistent-storage-app	qpg6100+debug	.bss	17778	17778
			.data	280	280
			.text	102704	102704
telink	lighting-app	tlsr9518adk80d	bss	71000	71000
			noinit	33216	33216
			text	458218	458218

26 builds

platform	target	config	section	`9b0afc4`	`cbe8747`	change
efr32	lighting-app	BRD4161A	.bss	118036	118036	0
			.data	1800	1800	0
			.text	782724	782724	0
	lock-app	BRD4161A	.bss	115892	115892	0
			.data	1760	1760	0
			.text	761868	761868	0
	window-app	BRD4161A	.bss	116212	116212	0
			.data	1764	1764	0
			.text	762792	762792	0
	lighting-app	BRD4161A+rpc	.bss	131364	131364	0
			.data	1852	1852	0
			.text	762468	762468	0
esp32	all-clusters-app	c3devkit	.dram0.bss	60296	60296	0
			.dram0.data	16192	16192	0
			.flash.rodata	198784	198784	0
			.flash.text	870186	870186	0
			.iram0.text	57330	57330	0
		m5stack	.dram0.bss	62792	62792	0
			.dram0.data	32084	32084	0
			.flash.rodata	207216	207216	0
			.flash.text	900547	900639	92
			.iram0.text	125115	125115	0
linux	all-clusters-app	debug	.bss	52176	52176	0
			.data	978	978	0
			.data.rel.ro	58688	58688	0
			.dynamic	592	592	0
			.got	4072	4072	0
			.init	27	27	0
			.init_array	512	512	0
			.rodata	135957	135957	0
			.text	`1327474`	`1327474`	0
	chip-tool	debug	.bss	17552	17552	0
			.data	1584	1584	0
			.data.rel.ro	84896	84896	0
			.dynamic	592	592	0
			.got	4336	4336	0
			.init	27	27	0
			.init_array	416	416	0
			.rodata	176928	176928	0
			.text	`2458309`	`2458309`	0
	ota-provider-app	debug	.bss	37472	37472	0
			.data	752	752	0
			.data.rel.ro	23176	23176	0
			.dynamic	592	592	0
			.got	4008	4008	0
			.init	27	27	0
			.init_array	440	440	0
			.rodata	109832	109832	0
			.text	1010322	1010322	0
	ota-requestor-app	debug	.bss	205728	205728	0
			.data	752	752	0
			.data.rel.ro	24488	24488	0
			.dynamic	592	592	0
			.got	4136	4136	0
			.init	27	27	0
			.init_array	512	512	0
			.rodata	128328	128328	0
			.text	1129634	1129634	0
	shell	debug	.bss	16072	16072	0
			.data	242	242	0
			.data.rel.ro	35120	35120	0
			.dynamic	592	592	0
			.got	3496	3496	0
			.init	27	27	0
			.init_array	336	336	0
			.rodata	71855	71855	0
			.text	569858	569858	0
	tv-app	debug	.bss	216400	216400	0
			.data	2032	2032	0
			.data.rel.ro	55872	55872	0
			.dynamic	592	592	0
			.got	4400	4400	0
			.init	27	27	0
			.init_array	608	608	0
			.rodata	151848	151848	0
			.text	1464210	1464210	0
	bridge-app	debug+rpc	.bss	52912	52912	0
			.data	976	976	0
			.data.rel.ro	25784	25784	0
			.dynamic	592	592	0
			.got	3944	3944	0
			.init	27	27	0
			.init_array	400	400	0
			.rodata	110452	110452	0
			.text	`1052229`	`1052229`	0
	lighting-app	debug+rpc	.bss	42232	42232	0
			.data	1106	1106	0
			.data.rel.ro	52496	52496	0
			.dynamic	608	608	0
			.got	4104	4104	0
			.init	27	27	0
			.init_array	528	528	0
			.rodata	127665	127665	0
			.text	`1253634`	`1253634`	0
mbed	lighting-app	CY8CPROTO_062_4343W+release	.bss	172164	172164	0
			.data	5464	5464	0
			.heap	858816	858816	0
			.text	`1219736`	`1219736`	0
	lock-app	CY8CPROTO_062_4343W+release	.bss	171084	171084	0
			.data	5432	5432	0
			.heap	859928	859928	0
			.text	1197632	1197632	0
nrfconnect	lighting-app	nrf52840dk_nrf52840	bss	112332	112332	0
			rodata	97688	97688	0
			text	577560	577560	0
	lock-app	nrf52840dk_nrf52840	bss	111348	111348	0
			rodata	94028	94028	0
			text	559060	559060	0
	pigweed-app	nrf52840dk_nrf52840	bss	51772	51772	0
			rodata	45772	45772	0
			text	339392	339392	0
	pump-app	nrf52840dk_nrf52840	bss	111416	111416	0
			rodata	95008	95008	0
			text	562216	562216	0
	pump-controller-app	nrf52840dk_nrf52840	bss	111356	111356	0
			rodata	94088	94088	0
			text	558852	558852	0
	shell	nrf52840dk_nrf52840	bss	107316	107316	0
			rodata	71640	71640	0
			text	519032	519032	0
	lighting-app	nrf52840dk_nrf52840+rpc	bss	108572	108572	0
			rodata	88480	88480	0
			text	550764	550764	0
		nrf5340dk_nrf5340_cpuapp	bss	113704	113704	0
			rodata	92932	92932	0
			text	507020	507020	0
	lock-app	nrf5340dk_nrf5340_cpuapp	bss	112720	112720	0
			rodata	89284	89284	0
			text	488516	488516	0
	shell	nrf5340dk_nrf5340_cpuapp	bss	108300	108300	0
			rodata	66284	66284	0
			text	439632	439632	0

austinh0 · 2021-10-18T19:47:35Z

Can you patch in this commit to fix the CI?

austinh0@0be80e7

pan-apple · 2021-10-18T20:17:03Z

Can you patch in this commit to fix the CI?

austinh0@0be80e7

Just saw this message. I have pushed a commit similar to what you mentioned in your comment. I'll pull in your commit as well to take care of the rest.

todo · 2021-10-18T20:22:17Z

(#8443): This method could benefit from a ChipDevice abstraction to hide the pointer

connectedhomeip/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java

Lines 110 to 115 in 8d0e838

    
              * <p>TODO(#8443): This method could benefit from a ChipDevice abstraction to hide the pointer 
        
              * passing. 
        
              */ 
        
             public void getConnectedDevicePointer(long nodeId, GetConnectedDeviceCallback callback) { 
        
               GetConnectedDeviceCallbackJni jniCallback = new GetConnectedDeviceCallbackJni(callback); 
        
               getConnectedDevicePointer(deviceControllerPtr, nodeId, jniCallback.getCallbackHandle());

This comment was generated by todo based on a `TODO` comment in `8d0e838` in #10612. cc @pan-apple.

github-actions · 2021-10-18T20:47:31Z

PR #10612: Size comparison from 9b0afc4 to 8d0e838

8 builds

platform	target	config	section	`9b0afc4`	`8d0e838`	change
k32w	lock-app	k32w061+debug	.bss	69052	69052	0
			.data	1864	1864	0
			.text	515320	515352	32
	shell	k32w061+debug	.bss	55080	55080	0
			.data	672	672	0
			.text	357300	357332	32
	lighting-app	k32w061+se05x+release	.bss	78576	78576	0
			.data	1900	1900	0
			.text	614088	614136	48
p6	lock-app	default	.bss	68216	68216	0
			.data	2416	2416	0
			.heap	962712	962712	0
			.text	1126424	1126472	48
qpg	lighting-app	qpg6100+debug	.bss	53552	53552	0
			.data	996	996	0
			.text	486400	486448	48
	lock-app	qpg6100+debug	.bss	52488	52488	0
			.data	952	952	0
			.text	462496	462520	24
	persistent-storage-app	qpg6100+debug	.bss	17778	17778	0
			.data	280	280	0
			.text	102704	102704	0
telink	lighting-app	tlsr9518adk80d	bss	71000	71000	0
			noinit	33216	33216	0
			text	458218	458220	2

Increases above 1.0% from 9b0afc4 to 8d0e838:

platform	target	config	section	`9b0afc4`	`8d0e838`	change	% change
linux	all-clusters-app	debug	.text	`1327474`	1344930	17456	1.3
	chip-tool	debug	.data.rel.ro	84896	86192	1296	1.5
			.text	`2458309`	`2859461`	401152	16.3

14 builds

platform	target	config	section	`9b0afc4`	`8d0e838`	change	% change
efr32	lighting-app	BRD4161A	.bss	118036	118036	0	0.0
			.data	1800	1800	0	0.0
			.text	782724	782788	64	0.0
	lock-app	BRD4161A	.bss	115892	115892	0	0.0
			.data	1760	1760	0	0.0
			.text	761868	761932	64	0.0
	window-app	BRD4161A	.bss	116212	116212	0	0.0
			.data	1764	1764	0	0.0
			.text	762792	762840	48	0.0
	lighting-app	BRD4161A+rpc	.bss	131364	131364	0	0.0
			.data	1852	1852	0	0.0
			.text	762468	762532	64	0.0
linux	all-clusters-app	debug	.bss	52176	52176	0	0.0
			.data	978	978	0	0.0
			.data.rel.ro	58688	58688	0	0.0
			.dynamic	592	592	0	0.0
			.got	4072	4072	0	0.0
			.init	27	27	0	0.0
			.init_array	512	512	0	0.0
			.rodata	135957	136149	192	0.1
			.text	`1327474`	1344930	17456	1.3
	chip-tool	debug	.bss	17552	17552	0	0.0
			.data	1584	1584	0	0.0
			.data.rel.ro	84896	86192	1296	1.5
			.dynamic	592	592	0	0.0
			.got	4336	4344	8	0.2
			.init	27	27	0	0.0
			.init_array	416	416	0	0.0
			.rodata	176928	178476	1548	0.9
			.text	`2458309`	`2859461`	401152	16.3
	ota-provider-app	debug	.bss	37472	37472	0	0.0
			.data	752	752	0	0.0
			.data.rel.ro	23176	23176	0	0.0
			.dynamic	592	592	0	0.0
			.got	4008	4008	0	0.0
			.init	27	27	0	0.0
			.init_array	440	440	0	0.0
			.rodata	109832	109832	0	0.0
			.text	1010322	1010354	32	0.0
	ota-requestor-app	debug	.bss	205728	205728	0	0.0
			.data	752	752	0	0.0
			.data.rel.ro	24488	24488	0	0.0
			.dynamic	592	592	0	0.0
			.got	4136	4136	0	0.0
			.init	27	27	0	0.0
			.init_array	512	512	0	0.0
			.rodata	128328	128328	0	0.0
			.text	1129634	1129666	32	0.0
	shell	debug	.bss	16072	16072	0	0.0
			.data	242	242	0	0.0
			.data.rel.ro	35120	35120	0	0.0
			.dynamic	592	592	0	0.0
			.got	3496	3496	0	0.0
			.init	27	27	0	0.0
			.init_array	336	336	0	0.0
			.rodata	71855	71855	0	0.0
			.text	569858	569858	0	0.0
	tv-app	debug	.bss	216400	216400	0	0.0
			.data	2032	2032	0	0.0
			.data.rel.ro	55872	55872	0	0.0
			.dynamic	592	592	0	0.0
			.got	4400	4400	0	0.0
			.init	27	27	0	0.0
			.init_array	608	608	0	0.0
			.rodata	151848	151848	0	0.0
			.text	1464210	1464242	32	0.0
	bridge-app	debug+rpc	.bss	52912	52912	0	0.0
			.data	976	976	0	0.0
			.data.rel.ro	25784	25784	0	0.0
			.dynamic	592	592	0	0.0
			.got	3944	3944	0	0.0
			.init	27	27	0	0.0
			.init_array	400	400	0	0.0
			.rodata	110452	110452	0	0.0
			.text	`1052229`	1052389	160	0.0
	lighting-app	debug+rpc	.bss	42232	42232	0	0.0
			.data	1106	1106	0	0.0
			.data.rel.ro	52496	52496	0	0.0
			.dynamic	608	608	0	0.0
			.got	4104	4104	0	0.0
			.init	27	27	0	0.0
			.init_array	528	528	0	0.0
			.rodata	127665	127665	0	0.0
			.text	`1253634`	`1253666`	32	0.0
mbed	lighting-app	CY8CPROTO_062_4343W+release	.bss	172164	172164	0	0.0
			.data	5464	5464	0	0.0
			.heap	858816	858816	0	0.0
			.text	`1219736`	1219800	64	0.0
	lock-app	CY8CPROTO_062_4343W+release	.bss	171084	171084	0	0.0
			.data	5432	5432	0	0.0
			.heap	859928	859928	0	0.0
			.text	1197632	1197632	0	0.0

2 builds

platform	target	config	section	`9b0afc4`	`8d0e838`	change	% change
esp32	all-clusters-app	c3devkit	.dram0.bss	60296	60296	0	0.0
			.dram0.data	16192	16192	0	0.0
			.flash.rodata	198784	198896	112	0.1
			.flash.text	870186	872534	2348	0.3
			.iram0.text	57330	57330	0	0.0
		m5stack	.dram0.bss	62792	62792	0	0.0
			.dram0.data	32084	32084	0	0.0
			.flash.rodata	207216	207320	104	0.1
			.flash.text	900547	902723	2176	0.2
			.iram0.text	125115	125115	0	0.0

10 builds

platform	target	config	section	`9b0afc4`	`8d0e838`	change
nrfconnect	lighting-app	nrf52840dk_nrf52840	bss	112332	112332	0
			rodata	97688	97688	0
			text	577560	577568	8
	lock-app	nrf52840dk_nrf52840	bss	111348	111348	0
			rodata	94028	94028	0
			text	559060	559068	8
	pigweed-app	nrf52840dk_nrf52840	bss	51772	51772	0
			rodata	45772	45772	0
			text	339392	339392	0
	pump-app	nrf52840dk_nrf52840	bss	111416	111416	0
			rodata	95008	95008	0
			text	562216	562224	8
	pump-controller-app	nrf52840dk_nrf52840	bss	111356	111356	0
			rodata	94088	94088	0
			text	558852	558860	8
	shell	nrf52840dk_nrf52840	bss	107316	107316	0
			rodata	71640	71640	0
			text	519032	519032	0
	lighting-app	nrf52840dk_nrf52840+rpc	bss	108572	108572	0
			rodata	88480	88480	0
			text	550764	550772	8
		nrf5340dk_nrf5340_cpuapp	bss	113704	113704	0
			rodata	92932	92932	0
			text	507020	507032	12
	lock-app	nrf5340dk_nrf5340_cpuapp	bss	112720	112720	0
			rodata	89284	89284	0
			text	488516	488528	12
	shell	nrf5340dk_nrf5340_cpuapp	bss	108300	108300	0
			rodata	66284	66284	0
			text	439632	439632	0

…0612) * Reduce usage of GetDevice() API in Android controller * Set PeerId correctly * remove remaining use of GetDevice(), GetCHIPDevice() and getDevicePointer() * add suspend * more build fixes

boring-cyborg bot added the controller label Oct 18, 2021

pullapprove bot requested a review from yufengwangca October 18, 2021 14:57

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

msandstedt reviewed Oct 18, 2021

View reviewed changes

src/controller/java/CHIPDeviceController-JNI.cpp Outdated Show resolved Hide resolved

pan-apple added 2 commits October 18, 2021 08:52

Reduce usage of GetDevice() API in Android controller

dcd3a90

Set PeerId correctly

620937b

msandstedt approved these changes Oct 18, 2021

View reviewed changes

remove remaining use of GetDevice(), GetCHIPDevice() and getDevicePoi…

cbe8747

…nter()

pan-apple force-pushed the android-controller branch from 029d722 to cbe8747 Compare October 18, 2021 15:58

boring-cyborg bot added the android label Oct 18, 2021

pan-apple requested a review from austinh0 October 18, 2021 15:59

add suspend

963d802

more build fixes

8d0e838

pan-apple changed the title ~~Reduce usage of GetDevice() API in Android controller~~ Remove usage of GetDevice() API in Android controller Oct 18, 2021

austinh0 approved these changes Oct 18, 2021

View reviewed changes

austinh0 added the technical review - DONE label Oct 18, 2021

woody-apple approved these changes Oct 18, 2021

View reviewed changes

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

pan-apple merged commit 05c3a35 into project-chip:master Oct 18, 2021

pan-apple deleted the android-controller branch October 18, 2021 22:23

todo bot mentioned this pull request Oct 18, 2021

(#8443): This method could benefit from a ChipDevice abstraction to hide the poi... #10632

Closed

austinh0 mentioned this pull request Oct 19, 2021

[Android] Fix build by changing computePaseVerifier to take device pointer #10672

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove usage of GetDevice() API in Android controller #10612

Remove usage of GetDevice() API in Android controller #10612

pan-apple commented Oct 18, 2021

github-actions bot commented Oct 18, 2021 •

edited

Loading

pan-apple commented Oct 18, 2021

github-actions bot commented Oct 18, 2021 •

edited

Loading

austinh0 commented Oct 18, 2021

pan-apple commented Oct 18, 2021

todo bot commented Oct 18, 2021

github-actions bot commented Oct 18, 2021 •

edited

Loading

Remove usage of GetDevice() API in Android controller #10612

Remove usage of GetDevice() API in Android controller #10612

Conversation

pan-apple commented Oct 18, 2021

Problem

Change overview

Testing

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

pan-apple commented Oct 18, 2021

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

austinh0 commented Oct 18, 2021

pan-apple commented Oct 18, 2021

todo bot commented Oct 18, 2021

(#8443): This method could benefit from a ChipDevice abstraction to hide the pointer

This comment was generated by todo based on a TODO comment in 8d0e838 in #10612. cc @pan-apple.

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

github-actions bot commented Oct 18, 2021 •

edited

Loading

github-actions bot commented Oct 18, 2021 •

edited

Loading

This comment was generated by todo based on a `TODO` comment in `8d0e838` in #10612. cc @pan-apple.

github-actions bot commented Oct 18, 2021 •

edited

Loading