Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the CHIPClusters ReadAttribute* APIs. #12652

Merged

Conversation

bzbarsky-apple
Copy link
Contributor

They were unused except by chip-tool. chip-tool is switched to use
the type-safe read API.

Problem

Command-line chip-tool can't read nullable attributes properly (i.e. if the value is null). And it's using the CHIPClusters APIs that nothing else uses.

Change overview

See above.

Testing

Manually tested a read of a null-valued attribute by hacking chip-tool to write a null value to it (which does not work normally work). Tested some reads of non-null attributes as well.

They were unused except by chip-tool.  chip-tool is switched to use
the type-safe read API.
@github-actions
Copy link

github-actions bot commented Dec 7, 2021

PR #12652: Size comparison from 5c70530 to 128fad7

Increases above 0.2%:

platform target config section 5c70530 128fad7 change % change
linux chip-tool debug (read/write) 200040 201512 1472 0.7
.data.rel.ro 158600 160072 1472 0.9
.rodata 313777 334449 20672 6.6
Increases (1 build for linux)
platform target config section 5c70530 128fad7 change % change
linux chip-tool debug (read only) 6651717 6661157 9440 0.1
(read/write) 200040 201512 1472 0.7
.data.rel.ro 158600 160072 1472 0.9
.rodata 313777 334449 20672 6.6
Decreases (4 builds for esp32, linux)
platform target config section 5c70530 128fad7 change % change
esp32 all-clusters-app m5stack (read only) 956475 955527 -948 -0.1
(read/write) 439528 439480 -48 -0.0
.flash.rodata 208132 208084 -48 -0.0
.flash.text 951091 950143 -948 -0.1
linux chip-tool debug .text 5933045 5918213 -14832 -0.2
ota-requestor-app debug (read only) 1516993 1512449 -4544 -0.3
(read/write) 78152 78120 -32 -0.0
.data.rel.ro 29720 29704 -16 -0.1
.text 1271362 1266866 -4496 -0.4
tv-app debug (read only) 2051945 2047377 -4568 -0.2
(read/write) 320320 320288 -32 -0.0
.data.rel.ro 64240 64208 -32 -0.0
.text 1721602 1717106 -4496 -0.3
Full report (39 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 5c70530 128fad7 change % change
efr32 lighting-app BRD4161A (read only) 797468 797468 0 0.0
(read/write) 120740 120740 0 0.0
.bss 118912 118912 0 0.0
.data 1828 1828 0 0.0
.text 797460 797460 0 0.0
BRD4161A+rpc (read only) 825436 825436 0 0.0
(read/write) 139048 139048 0 0.0
.bss 137112 137112 0 0.0
.data 1936 1936 0 0.0
.text 825428 825428 0 0.0
lock-app BRD4161A (read only) 772152 772152 0 0.0
(read/write) 118680 118680 0 0.0
.bss 116888 116888 0 0.0
.data 1788 1788 0 0.0
.text 772144 772144 0 0.0
window-app BRD4161A (read only) 774424 774424 0 0.0
(read/write) 118880 118880 0 0.0
.bss 117088 117088 0 0.0
.data 1792 1792 0 0.0
.text 774416 774416 0 0.0
esp32 all-clusters-app c3devkit (read only) 852210 852210 0 0.0
(read/write) 1295154 1295154 0 0.0
.dram0.bss 58232 58232 0 0.0
.dram0.data 14084 14084 0 0.0
.flash.rodata 169792 169792 0 0.0
.flash.text 852210 852210 0 0.0
.iram0.text 62076 62076 0 0.0
m5stack (read only) 956475 955527 -948 -0.1
(read/write) 439528 439480 -48 -0.0
.dram0.bss 65592 65592 0 0.0
.dram0.data 34016 34016 0 0.0
.flash.rodata 208132 208084 -48 -0.0
.flash.text 951091 950143 -948 -0.1
.iram0.text 123451 123451 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 731512 731512 0 0.0
.bss 79408 79408 0 0.0
.data 1860 1860 0 0.0
.text 644444 644444 0 0.0
lock-app k32w061+debug (read/write) 622212 622212 0 0.0
.bss 70072 70072 0 0.0
.data 1828 1828 0 0.0
.text 544512 544512 0 0.0
shell k32w061+debug (read/write) 688120 688120 0 0.0
.bss 81720 81720 0 0.0
.data 1800 1800 0 0.0
.text 598800 598800 0 0.0
linux all-clusters-app debug (read only) 1871081 1871081 0 0.0
(read/write) 124592 124592 0 0.0
.bss 50832 50832 0 0.0
.data 1120 1120 0 0.0
.data.rel.ro 67216 67216 0 0.0
.dynamic 592 592 0 0.0
.got 4120 4120 0 0.0
.init 27 27 0 0.0
.init_array 696 696 0 0.0
.rodata 153717 153717 0 0.0
.text 1574066 1574066 0 0.0
bridge-app debug+rpc (read only) 1450101 1450101 0 0.0
(read/write) 74904 74904 0 0.0
.bss 36464 36464 0 0.0
.data 1728 1728 0 0.0
.data.rel.ro 31632 31632 0 0.0
.dynamic 592 592 0 0.0
.got 3992 3992 0 0.0
.init 27 27 0 0.0
.init_array 480 480 0 0.0
.rodata 122692 122692 0 0.0
.text 1222341 1222341 0 0.0
chip-tool debug (read only) 6651717 6661157 9440 0.1
(read/write) 200040 201512 1472 0.7
.bss 34728 34728 0 0.0
.data 1024 1024 0 0.0
.data.rel.ro 158600 160072 1472 0.9
.dynamic 592 592 0 0.0
.got 4496 4496 0 0.0
.init 27 27 0 0.0
.init_array 568 568 0 0.0
.rodata 313777 334449 20672 6.6
.text 5933045 5918213 -14832 -0.2
lighting-app debug+rpc (read only) 1735881 1735881 0 0.0
(read/write) 107968 107968 0 0.0
.bss 42160 42160 0 0.0
.data 1280 1280 0 0.0
.data.rel.ro 59136 59136 0 0.0
.dynamic 608 608 0 0.0
.got 4144 4144 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 143281 143281 0 0.0
.text 1449762 1449762 0 0.0
ota-provider-app debug (read only) 1407641 1407641 0 0.0
(read/write) 73104 73104 0 0.0
.bss 39040 39040 0 0.0
.data 928 928 0 0.0
.data.rel.ro 27944 27944 0 0.0
.dynamic 592 592 0 0.0
.got 4056 4056 0 0.0
.init 27 27 0 0.0
.init_array 520 520 0 0.0
.rodata 124078 124078 0 0.0
.text 1178706 1178706 0 0.0
ota-requestor-app debug (read only) 1516993 1512449 -4544 -0.3
(read/write) 78152 78120 -32 -0.0
.bss 42208 42208 0 0.0
.data 992 992 0 0.0
.data.rel.ro 29720 29704 -16 -0.1
.dynamic 592 592 0 0.0
.got 4064 4064 0 0.0
.init 27 27 0 0.0
.init_array 544 544 0 0.0
.rodata 136823 136823 0 0.0
.text 1271362 1266866 -4496 -0.4
shell debug (read only) 823577 823577 0 0.0
(read/write) 60616 60616 0 0.0
.bss 16936 16936 0 0.0
.data 256 256 0 0.0
.data.rel.ro 38936 38936 0 0.0
.dynamic 592 592 0 0.0
.got 3520 3520 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 84754 84754 0 0.0
.text 631986 631986 0 0.0
tv-app debug (read only) 2051945 2047377 -4568 -0.2
(read/write) 320320 320288 -32 -0.0
.bss 247480 247480 0 0.0
.data 2768 2768 0 0.0
.data.rel.ro 64240 64208 -32 -0.0
.dynamic 592 592 0 0.0
.got 4456 4456 0 0.0
.init 27 27 0 0.0
.init_array 736 736 0 0.0
.rodata 175436 175436 0 0.0
.text 1721602 1717106 -4496 -0.3
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2312128 2312128 0 0.0
.bss 181412 181412 0 0.0
.data 5192 5192 0 0.0
.heap 849840 849840 0 0.0
.text 1274704 1274704 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2300032 2300032 0 0.0
.bss 173400 173400 0 0.0
.data 5496 5496 0 0.0
.heap 857552 857552 0 0.0
.text 1262632 1262632 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2273072 2273072 0 0.0
.bss 172440 172440 0 0.0
.data 5496 5496 0 0.0
.heap 858512 858512 0 0.0
.text 1235672 1235672 0 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1140008 1140008 0 0.0
.bss 11756 11756 0 0.0
.data 4376 4376 0 0.0
.heap 1020312 1020312 0 0.0
.text 103392 103392 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2047472 2047472 0 0.0
.bss 156732 156732 0 0.0
.data 4872 4872 0 0.0
.heap 874840 874840 0 0.0
.text 1010072 1010072 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 894539 894539 0 0.0
bss 113852 113852 0 0.0
rodata 99676 99676 0 0.0
text 605460 605460 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 857915 857915 0 0.0
bss 110200 110200 0 0.0
rodata 91036 91036 0 0.0
text 580456 580456 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 820438 820438 0 0.0
bss 115224 115224 0 0.0
rodata 94936 94936 0 0.0
text 535784 535784 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 867123 867123 0 0.0
bss 111112 111112 0 0.0
rodata 95796 95796 0 0.0
text 584832 584832 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 793258 793258 0 0.0
bss 112524 112524 0 0.0
rodata 91084 91084 0 0.0
text 515252 515252 0 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 497463 497463 0 0.0
bss 51820 51820 0 0.0
rodata 45852 45852 0 0.0
text 339492 339492 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 871859 871859 0 0.0
bss 111024 111024 0 0.0
rodata 97148 97148 0 0.0
text 588232 588232 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 865083 865083 0 0.0
bss 110904 110904 0 0.0
rodata 95284 95284 0 0.0
text 583440 583440 0 0.0
shell nrf52840dk_nrf52840 (read/write) 779939 779939 0 0.0
bss 109696 109696 0 0.0
rodata 73792 73792 0 0.0
text 521948 521948 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 694966 694966 0 0.0
bss 110680 110680 0 0.0
rodata 68432 68432 0 0.0
text 442548 442548 0 0.0
p6 all-clusters-app default (read/write) 2346536 2346536 0 0.0
.bss 107692 107692 0 0.0
.data 2464 2464 0 0.0
.heap 923184 923184 0 0.0
.text 1304800 1304800 0 0.0
light-app default (read/write) 2283768 2283768 0 0.0
.bss 98632 98632 0 0.0
.data 2336 2336 0 0.0
.heap 932376 932376 0 0.0
.text 1242032 1242032 0 0.0
lock-app default (read/write) 2259920 2259920 0 0.0
.bss 97512 97512 0 0.0
.data 2296 2296 0 0.0
.heap 933536 933536 0 0.0
.text 1218184 1218184 0 0.0
qpg lighting-app qpg6100+debug (read only) 513204 513204 0 0.0
(read/write) 122332 122332 0 0.0
.bss 80360 80360 0 0.0
.data 964 964 0 0.0
.text 507884 507884 0 0.0
lock-app qpg6100+debug (read only) 487508 487508 0 0.0
(read/write) 122336 122336 0 0.0
.bss 79496 79496 0 0.0
.data 920 920 0 0.0
.text 482188 482188 0 0.0
persistent-storage-app qpg6100+debug (read only) 108224 108224 0 0.0
(read/write) 122332 122332 0 0.0
.bss 36696 36696 0 0.0
.data 292 292 0 0.0
.text 102904 102904 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 798310 798310 0 0.0
bss 80428 80428 0 0.0
noinit 37160 37160 0 0.0
text 557916 557916 0 0.0

Copy link
Contributor

@woody-apple woody-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fast tracking, given this is updating a test tool

@bzbarsky-apple bzbarsky-apple merged commit 21392ef into project-chip:master Dec 7, 2021
@bzbarsky-apple bzbarsky-apple deleted the remove-old-read-api branch December 7, 2021 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants