Add Android tv server #11245

xylophone21 · 2021-10-30T07:27:02Z

Problem

Old Tv app is for Linux, not android, but for all android tv, tv features those be controlled are in android word

Change overview

Added a tv app for android

Testing

Baisc test by chip-device-ctrl python

xylophone21 · 2021-10-30T12:44:45Z

Any one could help to have a look at the 2 CI issues? Thanks

Zap issue. I know I should modify examples/tv-app/tv-common/tv-app.zap instead of zzz_generated/tv-app/zap-generated/CHIPClusters.cpp, but I do not know how to do it. I added 2 empty functions in CHIPClusters.cpp to fix link issue. I found that zzz_generated/tv-app/zap-generated deps src/controller, while src/controller deps zzz_generated/controller-clusters, so I have to add some controller-clusters functions to pass the linke
Test Suites issue. Seems it is because I changed the placed to init mChipStackLock and added some status check code at GenericPlatformManagerImpl_POSIX.cpp, but I do know why this check caused error. I added these code here because I found that some _LockChipStack is called before _InitChipStack, and some unlock is called when is not locked. ( revert these change doest not effect my PR, but I do not think hide a issue is a good idea )

bzbarsky-apple · 2021-10-30T14:22:52Z

I should modify examples/tv-app/tv-common/tv-app.zap instead of zzz_generated/tv-app/zap-generated/CHIPClusters.cpp, but I do not know how to do it.

The simplest way, but takes some time to run, is ./scripts/tools/zap_regen_all.py.

To just regenerate the CHIPClusters.cpp and CHIPClusters.h and other bits in zzz_generated/tv-app/zap-generated, you can run:

./scripts/tools/zap/generate.py -t src/app/zap-templates/app-templates.json -o zzz_generated/tv-app/zap-generated examples/tv-app/tv-common/tv-app.zap

and generally speaking you can find out which template (-t argument) to use by doing git grep CHIPClusters.cpp | grep json or equivalent for the filename you want to regenerate and seeing which template file is involved. The .zap file to use varies depending on which app you need to generate for, and the -o value depends on where you want things to land in the tree.

You will also want to rebase on tip to fix the ClusterInfoMapping.java bit in the ZAP job.

bzbarsky-apple · 2021-10-30T14:32:14Z

but I do know why this check caused error

Did you read the TSan output in the CI job? Your code is reading mChipStackIsLocked while not holding the lock, which ends up with a data race: one thread is reading the boolean (not holding the lock) at the same time as another thread is writing the boolean (while holding the lock). Which means that the answer you will get from the read will be random depending on how the threads race against each other.

In general, I don't understand what those changes are trying to do. Obviously mChipStackIsLocked could be true when _LockChipStack is called! That would indicate that some other thread is holding the lock, which is why we need a lock in the first place.

Now unlocking when not locked is in fact weird, and the check in _UnlockChipStack makes sense to add. But the checks being added in _TryLockChipStack and _LockChipStack make no sense.

I found that some _LockChipStack is called before _InitChipStack

So the thing we should really be checking in _LockChipStack and _TryLockChipStack is that the stack is initialized, right?

and some unlock is called when is not locked.

Stack trace? And steps to reproduce? Please file an issue with this information and @-mention me in it.

xylophone21 · 2021-11-01T13:57:26Z

but I do know why this check caused error

Did you read the TSan output in the CI job? Your code is reading mChipStackIsLocked while not holding the lock, which ends up with a data race: one thread is reading the boolean (not holding the lock) at the same time as another thread is writing the boolean (while holding the lock). Which means that the answer you will get from the read will be random depending on how the threads race against each other.

In general, I don't understand what those changes are trying to do. Obviously mChipStackIsLocked could be true when _LockChipStack is called! That would indicate that some other thread is holding the lock, which is why we need a lock in the first place.

Now unlocking when not locked is in fact weird, and the check in _UnlockChipStack makes sense to add. But the checks being added in _TryLockChipStack and _LockChipStack make no sense.

I found that some _LockChipStack is called before _InitChipStack

So the thing we should really be checking in _LockChipStack and _TryLockChipStack is that the stack is initialized, right?

and some unlock is called when is not locked.

Stack trace? And steps to reproduce? Please file an issue with this information and @-mention me in it.

Yes, that make sense, I debuged into this today, and found that 'unlock is called when is not locked' is in my pre code which is fixed now, so it is no problem now, thanks.

xylophone21 · 2021-11-01T14:04:33Z

I should modify examples/tv-app/tv-common/tv-app.zap instead of zzz_generated/tv-app/zap-generated/CHIPClusters.cpp, but I do not know how to do it.

The simplest way, but takes some time to run, is ./scripts/tools/zap_regen_all.py.

To just regenerate the CHIPClusters.cpp and CHIPClusters.h and other bits in zzz_generated/tv-app/zap-generated, you can run:
./scripts/tools/zap/generate.py -t src/app/zap-templates/app-templates.json -o zzz_generated/tv-app/zap-generated examples/tv-app/tv-common/tv-app.zap
and generally speaking you can find out which template (-t argument) to use by doing git grep CHIPClusters.cpp | grep json or equivalent for the filename you want to regenerate and seeing which template file is involved. The .zap file to use varies depending on which app you need to generate for, and the -o value depends on where you want things to land in the tree.

You will also want to rebase on tip to fix the ClusterInfoMapping.java bit in the ZAP job.

Yes, I know it is from tv-app.zap and app-templates.json, but any document about zap format? Or I need to read its code?

While I debuged on this issue today and found that:

Why I need to added AdministratorCommissioningCluster in tv-app/CHIPClusters.h is not because I need it, but because of tv-zap -> src/controller/CHIPDevice.cpp::Device::OpenPairingWindow --> OpenCommissioningWindow --> cluster.OpenCommissioningWindow. Without it I met link issues
I tried to comment Device::OpenPairingWindow and Device::OpenCommissioningWindow, them the link issue is fixed. It means that in fact no one are using these function, but '--gc-sections' seems does not work. I think it is the root case, and the right way is fix here. But I checked a lot, '--gc-sections' and '-ffunction-sections -fdata-sections' are all included in commands. Am I missed anything?

bzbarsky-apple · 2021-11-01T16:38:02Z

Yes, I know it is from tv-app.zap and app-templates.json, but any document about zap format? Or I need to read its code?

The .zap file is representing whatever was selected in the UI. You can run the UI as:

./scripts/tools/zap/run_zaptool.sh examples/tv-app/tv-common/tv-app.zap

Presumably you would want to enable the client side of the administrator commissioning cluster there, and enable these commands.

Without it I met link issues

Right, so your real issue is that Device::OpenPairingWindow is linked in and then pulls in all this other stuff.

but '--gc-sections' seems does not work.

This is on Android? Is that doing a static link or dynamic link? --gc-sections is effectively a no-op for dynamic linking; it only does something useful if you do static linking..

xylophone21 · 2021-11-02T06:48:55Z

dynamic

Yes, it is android.

Yes, I know it is from tv-app.zap and app-templates.json, but any document about zap format? Or I need to read its code?

The .zap file is representing whatever was selected in the UI. You can run the UI as:
./scripts/tools/zap/run_zaptool.sh examples/tv-app/tv-common/tv-app.zap
Presumably you would want to enable the client side of the administrator commissioning cluster there, and enable these commands.

Without it I met link issues

Right, so your real issue is that Device::OpenPairingWindow is linked in and then pulls in all this other stuff.

but '--gc-sections' seems does not work.

This is on Android? Is that doing a static link or dynamic link? --gc-sections is effectively a no-op for dynamic linking; it only does something useful if you do static linking..

Yes, android, I am doing some test on --gc-sections, looks you are right, it is not work on dynamic linking.

xylophone21 · 2021-11-02T12:24:35Z

Yes, I know it is from tv-app.zap and app-templates.json, but any document about zap format? Or I need to read its code?

The .zap file is representing whatever was selected in the UI. You can run the UI as:
./scripts/tools/zap/run_zaptool.sh examples/tv-app/tv-common/tv-app.zap

wow, it is amazing, thanks.

xylophone21 · 2021-11-02T13:32:43Z

Yes, I know it is from tv-app.zap and app-templates.json, but any document about zap format? Or I need to read its code?

The .zap file is representing whatever was selected in the UI. You can run the UI as:
./scripts/tools/zap/run_zaptool.sh examples/tv-app/tv-common/tv-app.zap
Presumably you would want to enable the client side of the administrator commissioning cluster there, and enable these commands.

Without it I met link issues

Right, so your real issue is that Device::OpenPairingWindow is linked in and then pulls in all this other stuff.

but '--gc-sections' seems does not work.

This is on Android? Is that doing a static link or dynamic link? --gc-sections is effectively a no-op for dynamic linking; it only does something useful if you do static linking..

I think I found some way to fix it, I added '-fvisibility=hidden' to all symbols and declare jni functions to JNIEXPORT (visibility=default)

…fined issue

github-actions · 2021-11-02T13:51:22Z

PR #11245: Size comparison from 9b26453 to 5289a6f

Increases (8 builds for linux)

platform	target	config	section	`9b26453`	`5289a6f`	change
linux	all-clusters-app	debug	(read only)	1684089	1684169	80
			.rodata	137973	138005	32
			.text	1413058	1413106	48
	bridge-app	debug+rpc	(read only)	1277557	1277637	80
			.rodata	109604	109636	32
			.text	1072709	`1072757`	48
	chip-tool	debug	(read only)	`4307845`	4307925	80
			.rodata	217360	217392	32
			.text	3824997	`3825045`	48
	lighting-app	debug+rpc	(read only)	`1536529`	`1536609`	80
			.rodata	127761	127793	32
			.text	1276386	1276434	48
	ota-provider-app	debug	(read only)	1238441	1238505	64
			.rodata	110600	110632	32
			.text	1031714	1031746	32
	ota-requestor-app	debug	(read only)	1292785	1292865	80
			.rodata	121568	121600	32
			.text	1073426	1073474	48
	shell	debug	(read only)	785041	785121	80
			.rodata	77295	77327	32
			.text	606210	606258	48
	tv-app	debug	(read only)	1771401	1771481	80
			.rodata	153304	153336	32
			.text	1476194	1476242	48

Full report (37 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)

platform	target	config	section	`9b26453`	`5289a6f`	change
efr32	lighting-app	BRD4161A	(read only)	735256	735256	0
			(read/write)	114444	114444	0
			.bss	112692	112692	0
			.data	1752	1752	0
			.text	735248	735248	0
		BRD4161A+rpc	(read only)	722696	722696	0
			(read/write)	131052	131052	0
			.bss	129196	129196	0
			.data	1852	1852	0
			.text	722688	722688	0
	lock-app	BRD4161A	(read only)	714540	714540	0
			(read/write)	112260	112260	0
			.bss	110548	110548	0
			.data	1712	1712	0
			.text	714532	714532	0
	window-app	BRD4161A	(read only)	715452	715452	0
			(read/write)	112584	112584	0
			.bss	110868	110868	0
			.data	1716	1716	0
			.text	715444	715444	0
esp32	all-clusters-app	c3devkit	(read only)	880186	880186	0
			(read/write)	1307544	1307544	0
			.dram0.bss	58424	58424	0
			.dram0.data	16464	16464	0
			.flash.rodata	199416	199416	0
			.flash.text	880186	880186	0
			.iram0.text	57554	57554	0
		m5stack	(read only)	911131	911131	0
			(read/write)	427304	427304	0
			.dram0.bss	60920	60920	0
			.dram0.data	32100	32100	0
			.flash.rodata	208120	208120	0
			.flash.text	911131	911131	0
			.iram0.text	125115	125115	0
k32w	lighting-app	k32w061+se05x+release	(read/write)	698120	698120	0
			.bss	77688	77688	0
			.data	1900	1900	0
			.text	612732	612732	0
	lock-app	k32w061+debug	(read/write)	590412	590412	0
			.bss	68188	68188	0
			.data	1864	1864	0
			.text	514560	514560	0
	shell	k32w061+debug	(read/write)	424772	424772	0
			.bss	63280	63280	0
			.data	672	672	0
			.text	359116	359116	0
linux	all-clusters-app	debug	(read only)	1684089	1684169	80
			(read/write)	118992	118992	0
			.bss	50608	50608	0
			.data	1010	1010	0
			.data.rel.ro	62112	62112	0
			.dynamic	592	592	0
			.got	4088	4088	0
			.init	27	27	0
			.init_array	536	536	0
			.rodata	137973	138005	32
			.text	1413058	1413106	48
	bridge-app	debug+rpc	(read only)	1277557	1277637	80
			(read/write)	84104	84104	0
			.bss	50768	50768	0
			.data	976	976	0
			.data.rel.ro	27368	27368	0
			.dynamic	592	592	0
			.got	3952	3952	0
			.init	27	27	0
			.init_array	400	400	0
			.rodata	109604	109636	32
			.text	1072709	`1072757`	48
	chip-tool	debug	(read only)	`4307845`	4307925	80
			(read/write)	123488	123488	0
			.bss	17680	17680	0
			.data	1584	1584	0
			.data.rel.ro	98800	98800	0
			.dynamic	592	592	0
			.got	4368	4368	0
			.init	27	27	0
			.init_array	424	424	0
			.rodata	217360	217392	32
			.text	3824997	`3825045`	48
	lighting-app	debug+rpc	(read only)	`1536529`	`1536609`	80
			(read/write)	100976	100976	0
			.bss	40152	40152	0
			.data	1106	1106	0
			.data.rel.ro	54448	54448	0
			.dynamic	608	608	0
			.got	4112	4112	0
			.init	27	27	0
			.init_array	528	528	0
			.rodata	127761	127793	32
			.text	1276386	1276434	48
	ota-provider-app	debug	(read only)	1238441	1238505	64
			(read/write)	67072	67072	0
			.bss	36608	36608	0
			.data	752	752	0
			.data.rel.ro	24616	24616	0
			.dynamic	592	592	0
			.got	4016	4016	0
			.init	27	27	0
			.init_array	440	440	0
			.rodata	110600	110632	32
			.text	1031714	1031746	32
	ota-requestor-app	debug	(read only)	1292785	1292865	80
			(read/write)	76392	76392	0
			.bss	44864	44864	0
			.data	816	816	0
			.data.rel.ro	25576	25576	0
			.dynamic	592	592	0
			.got	3984	3984	0
			.init	27	27	0
			.init_array	512	512	0
			.rodata	121568	121600	32
			.text	1073426	1073474	48
	shell	debug	(read only)	785041	785121	80
			(read/write)	57664	57664	0
			.bss	16072	16072	0
			.data	242	242	0
			.data.rel.ro	36848	36848	0
			.dynamic	592	592	0
			.got	3528	3528	0
			.init	27	27	0
			.init_array	336	336	0
			.rodata	77295	77327	32
			.text	606210	606258	48
	tv-app	debug	(read only)	1771401	1771481	80
			(read/write)	288536	288536	0
			.bss	222192	222192	0
			.data	2032	2032	0
			.data.rel.ro	58672	58672	0
			.dynamic	592	592	0
			.got	4408	4408	0
			.init	27	27	0
			.init_array	608	608	0
			.rodata	153304	153336	32
			.text	1476194	1476242	48
mbed	all-clusters-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0
			(read/write)	2293608	2293608	0
			.bss	179796	179796	0
			.data	5216	5216	0
			.heap	851432	851432	0
			.text	1256208	1256208	0
	lighting-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0
			(read/write)	2272608	2272608	0
			.bss	171836	171836	0
			.data	5568	5568	0
			.heap	859040	859040	0
			.text	`1235208`	`1235208`	0
	lock-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0
			(read/write)	2250328	2250328	0
			.bss	170740	170740	0
			.data	5552	5552	0
			.heap	860152	860152	0
			.text	1212928	1212928	0
	pigweed-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0
			(read/write)	1139744	1139744	0
			.bss	11752	11752	0
			.data	4368	4368	0
			.heap	1020328	1020328	0
			.text	103128	103128	0
	shell	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0
			(read/write)	`2048328`	`2048328`	0
			.bss	156232	156232	0
			.data	4968	4968	0
			.heap	875248	875248	0
			.text	1010928	1010928	0
nrfconnect	lighting-app	nrf52840dk_nrf52840	(read/write)	860063	860063	0
			bss	111148	111148	0
			rodata	96360	96360	0
			text	576940	576940	0
		nrf52840dk_nrf52840+rpc	(read/write)	820887	820887	0
			bss	107392	107392	0
			rodata	87136	87136	0
			text	550112	550112	0
		nrf5340dk_nrf5340_cpuapp	(read/write)	785106	785106	0
			bss	112524	112524	0
			rodata	91604	91604	0
			text	506408	506408	0
	lock-app	nrf52840dk_nrf52840	(read/write)	836771	836771	0
			bss	110184	110184	0
			rodata	92716	92716	0
			text	558420	558420	0
		nrf5340dk_nrf5340_cpuapp	(read/write)	762066	762066	0
			bss	111596	111596	0
			rodata	88020	88020	0
			text	487980	487980	0
	pigweed-app	nrf52840dk_nrf52840	(read/write)	497323	497323	0
			bss	51824	51824	0
			rodata	45776	45776	0
			text	339436	339436	0
	pump-app	nrf52840dk_nrf52840	(read/write)	842879	842879	0
			bss	110320	110320	0
			rodata	94424	94424	0
			text	562600	562600	0
	pump-controller-app	nrf52840dk_nrf52840	(read/write)	836627	836627	0
			bss	110220	110220	0
			rodata	92716	92716	0
			text	558156	558156	0
	shell	nrf52840dk_nrf52840	(read/write)	775903	775903	0
			bss	109096	109096	0
			rodata	72404	72404	0
			text	519792	519792	0
		nrf5340dk_nrf5340_cpuapp	(read/write)	690906	690906	0
			bss	110080	110080	0
			rodata	67044	67044	0
			text	440400	440400	0
p6	lock-app	default	(read/write)	`2166400`	`2166400`	0
			.bss	66184	66184	0
			.data	2416	2416	0
			.heap	964744	964744	0
			.text	1124664	1124664	0
qpg	lighting-app	qpg6100+debug	(read only)	489560	489560	0
			(read/write)	114144	114144	0
			.bss	50320	50320	0
			.data	1000	1000	0
			.text	484240	484240	0
	lock-app	qpg6100+debug	(read only)	465888	465888	0
			(read/write)	114140	114140	0
			.bss	49272	49272	0
			.data	956	956	0
			.text	460568	460568	0
	persistent-storage-app	qpg6100+debug	(read only)	155820	155820	0
			(read/write)	114140	114140	0
			.bss	27752	27752	0
			.data	372	372	0
			.text	150500	150500	0
telink	lighting-app	tlsr9518adk80d	(read/write)	661470	661470	0
			bss	68960	68960	0
			noinit	33216	33216	0
			text	457194	457194	0

woody-apple · 2021-11-02T15:28:52Z

Fast tracking, given platform specific code.

* added app server jni * added getQrCodeFromPayload in SetupPayloadParser * added TVServer app for android * fix BLE error * fix ci issue * fix restyled errors * fix ci issue * fix Tests / Test Suites - Linux (tsan) ci issue * fix zap issue, we should not add unneed functions in zap but fix undefined issue Conflicts fixed: scripts/build/build/targets.py scripts/build/testdata/build_all_except_host.txt scripts/build/testdata/glob_star_targets_except_host.txt

* added app server jni * added getQrCodeFromPayload in SetupPayloadParser * added TVServer app for android * fix BLE error * fix ci issue * fix restyled errors * fix ci issue * fix Tests / Test Suites - Linux (tsan) ci issue * fix zap issue, we should not add unneed functions in zap but fix undefined issue

boring-cyborg bot added the examples label Oct 30, 2021

restyled-io bot mentioned this pull request Oct 30, 2021

Restyle Add Android tv server #11246

Closed

xylophone21 force-pushed the android-tvserver branch from 7fe42f0 to 0938e17 Compare October 30, 2021 07:29

xylophone21 force-pushed the android-tvserver branch from 8b6896c to e05557a Compare November 1, 2021 09:46

xylophone21 added 8 commits November 2, 2021 19:57

added app server jni

ccd7334

added getQrCodeFromPayload in SetupPayloadParser

969c8dd

added TVServer app for android

438f01a

fix BLE error

8c69a8b

fix ci issue

f02292d

fix restyled errors

93c8fda

fix ci issue

d5fb978

fix Tests / Test Suites - Linux (tsan) ci issue

8fdf550

xylophone21 force-pushed the android-tvserver branch from ea78d84 to 4b89008 Compare November 2, 2021 11:57

xylophone21 force-pushed the android-tvserver branch from 4b89008 to 7773687 Compare November 2, 2021 13:28

restyled-io bot mentioned this pull request Nov 2, 2021

Restyle Add Android tv server #11319

Closed

fix zap issue, we should not add unneed functions in zap but fix unde…

5289a6f

…fined issue

xylophone21 force-pushed the android-tvserver branch from 7773687 to 5289a6f Compare November 2, 2021 13:34

woody-apple approved these changes Nov 2, 2021

View reviewed changes

woody-apple added the fast track label Nov 2, 2021

woody-apple merged commit 62b337e into project-chip:master Nov 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Android tv server #11245

Add Android tv server #11245

xylophone21 commented Oct 30, 2021

xylophone21 commented Oct 30, 2021 •

edited

Loading

bzbarsky-apple commented Oct 30, 2021 •

edited

Loading

bzbarsky-apple commented Oct 30, 2021 •

edited

Loading

xylophone21 commented Nov 1, 2021

xylophone21 commented Nov 1, 2021

bzbarsky-apple commented Nov 1, 2021 •

edited

Loading

xylophone21 commented Nov 2, 2021

xylophone21 commented Nov 2, 2021

xylophone21 commented Nov 2, 2021

github-actions bot commented Nov 2, 2021 •

edited

Loading

woody-apple commented Nov 2, 2021

Add Android tv server #11245

Add Android tv server #11245

Conversation

xylophone21 commented Oct 30, 2021

Problem

Change overview

Testing

xylophone21 commented Oct 30, 2021 • edited Loading

bzbarsky-apple commented Oct 30, 2021 • edited Loading

bzbarsky-apple commented Oct 30, 2021 • edited Loading

xylophone21 commented Nov 1, 2021

xylophone21 commented Nov 1, 2021

bzbarsky-apple commented Nov 1, 2021 • edited Loading

xylophone21 commented Nov 2, 2021

xylophone21 commented Nov 2, 2021

xylophone21 commented Nov 2, 2021

github-actions bot commented Nov 2, 2021 • edited Loading

woody-apple commented Nov 2, 2021

xylophone21 commented Oct 30, 2021 •

edited

Loading

bzbarsky-apple commented Oct 30, 2021 •

edited

Loading

bzbarsky-apple commented Oct 30, 2021 •

edited

Loading

bzbarsky-apple commented Nov 1, 2021 •

edited

Loading

github-actions bot commented Nov 2, 2021 •

edited

Loading