WindowCovering: fix Limits checks + factorization #13729

jmeg-sfy · 2022-01-19T21:23:01Z

Problem

What is being fixed? Examples:

Goal: Lay down some small changes for next PRs

Fix : Limits checks for invalid Percenth100s
Simplification and factorization for incoming other PRs

Change overview

Add some Helper methods :

to check a Percenth100
compute a movement comparing target and current
factorize the conversion function using AbsolutLimits

Move HasFeature functions

Testing

How was this tested? (at least one bullet point required)

Test on EFR32 Window-app example
Passing actual YAML tests

jmeg-sfy · 2022-01-21T19:48:56Z

@bzbarsky-apple if you would like to have a look thanks , @eve-cxrp also thanks

src/app/clusters/window-covering-server/window-covering-server.cpp

src/app/clusters/window-covering-server/window-covering-server.h

- automatically set global field

github-actions · 2022-01-24T10:44:53Z

PR #13729: Size comparison from 5227f3e to 5c51952

Increases (4 builds for efr32, esp32, p6)

platform	target	config	section	`5227f3e`	`5c51952`	change
efr32	window-app	BRD4161A	(read only)	806476	806764	288
			.text	806468	806756	288
esp32	all-clusters-app	c3devkit	(read only)	918984	919012	28
			.flash.text	918984	919012	28
		m5stack	(read only)	967431	967459	28
			.flash.text	962047	962075	28
p6	all-clusters-app	default	(read/write)	2410664	2410680	16
			.text	1368928	1368944	16

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

platform	target	config	section	`5227f3e`	`5c51952`	change
cyw30739	light	cyw930739m2evb_01	(read/write)	574390	574390	0
			.app_xip_area	479404	479404	0
			.bss	77724	77724	0
			.data	604	604	0
			.rodata	0	0	0
			.text	0	0	0
efr32	lighting-app	BRD4161A	(read only)	835916	835916	0
			(read/write)	127244	127244	0
			.bss	125344	125344	0
			.data	1900	1900	0
			.text	835908	835908	0
		BRD4161A+rpc	(read only)	823336	823336	0
			(read/write)	143904	143904	0
			.bss	141904	141904	0
			.data	2000	2000	0
			.text	823328	823328	0
	window-app	BRD4161A	(read only)	806476	806764	288
			(read/write)	125812	125812	0
			.bss	123960	123960	0
			.data	1852	1852	0
			.text	806468	806756	288
esp32	all-clusters-app	c3devkit	(read only)	918984	919012	28
			(read/write)	1383434	1383434	0
			.dram0.bss	70720	70720	0
			.dram0.data	14252	14252	0
			.flash.rodata	179344	179344	0
			.flash.text	918984	919012	28
			.iram0.text	62056	62056	0
		m5stack	(read only)	967431	967459	28
			(read/write)	449852	449852	0
			.dram0.bss	75184	75184	0
			.dram0.data	34032	34032	0
			.flash.rodata	208508	208508	0
			.flash.text	962047	962075	28
			.iram0.text	123399	123399	0
k32w	light	k32w061+release	(read/write)	660588	660588	0
			.bss	76608	76608	0
			.data	1868	1868	0
			.text	576312	576312	0
	lock	k32w061+release	(read/write)	661312	661312	0
			.bss	76848	76848	0
			.data	1888	1888	0
			.text	576776	576776	0
linux	chip-tool-ipv6only	arm64	(read only)	8536964	8536964	0
			(read/write)	391009	391009	0
			.bss	56049	56049	0
			.data	1128	1128	0
			.data.rel.ro	261032	261032	0
			.dynamic	560	560	0
			.got	69056	69056	0
			.init	24	24	0
			.init_array	200	200	0
			.rodata	436908	436908	0
			.text	`7261028`	`7261028`	0
	thermostat-no-ble	arm64	(read only)	`2051916`	`2051916`	0
			(read/write)	145697	145697	0
			.bss	64785	64785	0
			.data	904	904	0
			.data.rel.ro	72984	72984	0
			.dynamic	560	560	0
			.got	4064	4064	0
			.init	24	24	0
			.init_array	320	320	0
			.rodata	130700	130700	0
			.text	`1706720`	`1706720`	0
mbed	all-clusters-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0
			(read/write)	2352184	2352184	0
			.bss	189428	189428	0
			.data	5304	5304	0
			.text	1314760	1314760	0
	lighting-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0
			(read/write)	2334408	2334408	0
			.bss	180960	180960	0
			.data	5584	5584	0
			.text	1297008	1297008	0
	lock-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0
			(read/write)	2304760	2304760	0
			.bss	179960	179960	0
			.data	5560	5560	0
			.text	1267360	1267360	0
	pigweed-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0
			(read/write)	1139712	1139712	0
			.bss	11756	11756	0
			.data	4368	4368	0
			.text	103096	103096	0
	shell	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0
			(read/write)	2054256	2054256	0
			.bss	156876	156876	0
			.data	4864	4864	0
			.text	1016856	1016856	0
nrfconnect	lighting-app	nrf52840dk_nrf52840	(read/write)	942879	942879	0
			bss	119044	119044	0
			rodata	108916	108916	0
			text	637312	637312	0
		nrf52840dk_nrf52840+rpc	(read/write)	928347	928347	0
			bss	116088	116088	0
			rodata	101368	101368	0
			text	632708	632708	0
		nrf52840dongle_nrf52840	(read/write)	993527	993527	0
			bss	121884	121884	0
			rodata	113672	113672	0
			text	669512	669512	0
		nrf5340dk_nrf5340_cpuapp	(read/write)	852706	852706	0
			bss	115828	115828	0
			rodata	102092	102092	0
			text	554232	554232	0
	lock-app	nrf52840dk_nrf52840	(read/write)	912223	912223	0
			bss	118196	118196	0
			rodata	103884	103884	0
			text	612756	612756	0
		nrf5340dk_nrf5340_cpuapp	(read/write)	822294	822294	0
			bss	115016	115016	0
			rodata	97112	97112	0
			text	529712	529712	0
	pigweed-app	nrf52840dk_nrf52840	(read/write)	541835	541835	0
			bss	52588	52588	0
			rodata	50104	50104	0
			text	376940	376940	0
	pump-app	nrf52840dk_nrf52840	(read/write)	915083	915083	0
			bss	117960	117960	0
			rodata	104248	104248	0
			text	615404	615404	0
	pump-controller-app	nrf52840dk_nrf52840	(read/write)	910267	910267	0
			bss	117984	117984	0
			rodata	103400	103400	0
			text	611428	611428	0
	shell	nrf52840dk_nrf52840	(read/write)	798527	798527	0
			bss	109776	109776	0
			rodata	78324	78324	0
			text	533928	533928	0
		nrf5340dk_nrf5340_cpuapp	(read/write)	711342	711342	0
			bss	107664	107664	0
			rodata	72624	72624	0
			text	451608	451608	0
p6	all-clusters-app	default	(read/write)	2410664	2410680	16
			.bss	117772	117772	0
			.data	2584	2584	0
			.text	1368928	1368944	16
	light-app	default	(read/write)	2330736	2330736	0
			.bss	105544	105544	0
			.data	2408	2408	0
			.text	1289000	1289000	0
	lock-app	default	(read/write)	2299752	2299752	0
			.bss	104392	104392	0
			.data	2360	2360	0
			.text	`1258016`	`1258016`	0
qpg	lighting-app	qpg6105+debug	(read only)	567476	567476	0
			(read/write)	146940	146940	0
			.bss	89688	89688	0
			.data	1064	1064	0
			.text	562156	562156	0
	lock-app	qpg6105+debug	(read only)	515904	515904	0
			(read/write)	146936	146936	0
			.bss	88256	88256	0
			.data	988	988	0
			.text	510584	510584	0
	persistent-storage-app	qpg6105+debug	(read only)	107140	107140	0
			(read/write)	146940	146940	0
			.bss	38504	38504	0
			.data	288	288	0
			.text	101820	101820	0
telink	lighting-app	tlsr9518adk80d	(read/write)	841718	841718	0
			bss	87492	87492	0
			noinit	37160	37160	0
			text	588346	588346	0

jmeg-sfy · 2022-01-24T19:13:14Z

@bzbarsky-apple

woody-apple

Fast tracking, given > 3 days of review. Comments look resolved too.

* DEV: Remove IsOpen IsClosed function and use CheckLimitState * DEV: Move HasFeature function for external usage * DEV: Factorize conversion function * DEV: Add IsPercent100thsValid check method * DEV: Add OperationalStatusSet Global field helpers function - automatically set global field * DEV: Add ComputeOperationalState function * Restyled by clang-format * DEV: Rename limits using "Over" to "Post" * DEV: Cleanup HasFeature function * DEV: Add comments to explain the convert function * DEV: Replace Limit Post by Past * Style: correct bad description of ConvertValue Co-authored-by: Restyled.io <commits@restyled.io>

boring-cyborg bot added app examples labels Jan 19, 2022

restyled-io bot mentioned this pull request Jan 19, 2022

Restyle WindowCovering: fix Limits checks + factorization #13730

Closed

jmartinez-silabs approved these changes Jan 20, 2022

View reviewed changes

andy31415 approved these changes Jan 21, 2022

View reviewed changes

jmeg-sfy requested review from bzbarsky-apple and a user January 21, 2022 08:37

bzbarsky-apple reviewed Jan 22, 2022

View reviewed changes

jmeg-sfy and others added 12 commits January 24, 2022 11:12

DEV: Remove IsOpen IsClosed function and use CheckLimitState

f7a2b22

DEV: Move HasFeature function for external usage

a297566

DEV: Factorize conversion function

334a1d3

DEV: Add IsPercent100thsValid check method

3ee4bd3

DEV: Add OperationalStatusSet Global field helpers function

570ec89

- automatically set global field

DEV: Add ComputeOperationalState function

4faedd0

Restyled by clang-format

7aeea79

DEV: Rename limits using "Over" to "Post"

3d5a233

DEV: Cleanup HasFeature function

90345fa

DEV: Add comments to explain the convert function

5cf5ba2

DEV: Replace Limit Post by Past

deca48c

Style: correct bad description of ConvertValue

5c51952

jmeg-sfy force-pushed the jmeg-sfy/window-covering-conversion branch from 42379bb to 5c51952 Compare January 24, 2022 10:30

woody-apple approved these changes Jan 26, 2022

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels Jan 26, 2022

woody-apple added fast track review - pending and removed review - approved labels Jan 26, 2022

woody-apple merged commit 1ccf850 into project-chip:master Jan 26, 2022

jmeg-sfy deleted the jmeg-sfy/window-covering-conversion branch January 26, 2022 12:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WindowCovering: fix Limits checks + factorization #13729

WindowCovering: fix Limits checks + factorization #13729

jmeg-sfy commented Jan 19, 2022

jmeg-sfy commented Jan 21, 2022

github-actions bot commented Jan 24, 2022 •

edited

Loading

jmeg-sfy commented Jan 24, 2022

woody-apple left a comment

WindowCovering: fix Limits checks + factorization #13729

WindowCovering: fix Limits checks + factorization #13729

Conversation

jmeg-sfy commented Jan 19, 2022

Problem

Change overview

Testing

jmeg-sfy commented Jan 21, 2022

github-actions bot commented Jan 24, 2022 • edited Loading

jmeg-sfy commented Jan 24, 2022

woody-apple left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 24, 2022 •

edited

Loading