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

Improve DecodableList when not in list #13607

Merged
merged 2 commits into from
Jan 17, 2022

Conversation

mlepage-google
Copy link
Contributor

@mlepage-google mlepage-google commented Jan 14, 2022

Problem

DecodableList doesn't behave well if you don't supply a reader
with a well-formed list.

Change overview

Add a ClearReader method, used by the constructor but also
reusable later, that initializes the internal reader to a
container-less state. Check for this container-less state in
ComputeSize and iterator methods such as Next.

Testing

Built and ran all-clusters-app on Linux with REPL to try null lists
as well as lists with items.

Spec treats empty lists and null valued lists equivalently.

However DecodableList expects a container, even if empty, and does
not support the case where there is no container (i.e. null valued
list).

Add support for null valued list (i.e. no container) by handling
it as a special case in the Decode function, the DecodableList,
and its Iterator. This is done using an "empty" mReader (with
container type "not specified") in the "no list" case.
@github-actions
Copy link

github-actions bot commented Jan 14, 2022

PR #13607: Size comparison from a11413e to 31637e3

Increases (24 builds for efr32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section a11413e 31637e3 change % change
efr32 lighting-app BRD4161A (read only) 834852 834916 64 0.0
.text 834844 834908 64 0.0
BRD4161A+rpc (read only) 822232 822296 64 0.0
.text 822224 822288 64 0.0
window-app BRD4161A (read only) 805424 805472 48 0.0
.text 805416 805464 48 0.0
k32w light k32w061+release (read/write) 657920 658000 80 0.0
.text 573132 573212 80 0.0
lock k32w061+release (read/write) 661796 661860 64 0.0
.text 576692 576756 64 0.0
linux chip-tool-ipv6only arm64 (read only) 8042100 8051044 8944 0.1
.text 6833684 6842628 8944 0.1
thermostat-no-ble arm64 (read only) 2042108 2043020 912 0.0
.text 1697632 1698544 912 0.1
mbed all-clusters-app CY8CPROTO_062_4343W+release (read/write) 2349856 2350560 704 0.0
.text 1312432 1313136 704 0.1
lighting-app CY8CPROTO_062_4343W+release (read/write) 2334504 2334632 128 0.0
.text 1297104 1297232 128 0.0
lock-app CY8CPROTO_062_4343W+release (read/write) 2304848 2304976 128 0.0
.text 1267448 1267576 128 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 943403 943499 96 0.0
text 637376 637468 92 0.0
nrf52840dk_nrf52840+rpc (read/write) 928883 928979 96 0.0
text 632772 632864 92 0.0
nrf52840dongle_nrf52840 (read/write) 994079 994175 96 0.0
text 669576 669668 92 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 853242 853322 80 0.0
text 554300 554392 92 0.0
lock-app nrf52840dk_nrf52840 (read/write) 912763 912827 64 0.0
text 612824 612880 56 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 822830 822878 48 0.0
text 529784 529836 52 0.0
pump-app nrf52840dk_nrf52840 (read/write) 915627 915675 48 0.0
text 615480 615532 52 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 910715 910763 48 0.0
text 611404 611460 56 0.0
p6 all-clusters-app default (read/write) 2404384 2405216 832 0.0
.text 1362648 1363480 832 0.1
light-app default (read/write) 2329688 2329752 64 0.0
.text 1287952 1288016 64 0.0
lock-app default (read/write) 2298712 2298744 32 0.0
.text 1256976 1257008 32 0.0
qpg lighting-app qpg6105+debug (read only) 563984 564080 96 0.0
.text 558664 558760 96 0.0
lock-app qpg6105+debug (read only) 515484 515548 64 0.0
.text 510164 510228 64 0.0
telink lighting-app tlsr9518adk80d (read/write) 840778 840970 192 0.0
text 587748 587940 192 0.0
Full report (30 builds for efr32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section a11413e 31637e3 change % change
efr32 lighting-app BRD4161A (read only) 834852 834916 64 0.0
(read/write) 127628 127628 0 0.0
.bss 125744 125744 0 0.0
.data 1884 1884 0 0.0
.text 834844 834908 64 0.0
BRD4161A+rpc (read only) 822232 822296 64 0.0
(read/write) 144288 144288 0 0.0
.bss 142304 142304 0 0.0
.data 1984 1984 0 0.0
.text 822224 822288 64 0.0
window-app BRD4161A (read only) 805424 805472 48 0.0
(read/write) 126320 126320 0 0.0
.bss 124480 124480 0 0.0
.data 1836 1836 0 0.0
.text 805416 805464 48 0.0
k32w light k32w061+release (read/write) 657920 658000 80 0.0
.bss 77136 77136 0 0.0
.data 1852 1852 0 0.0
.text 573132 573212 80 0.0
lock k32w061+release (read/write) 661796 661860 64 0.0
.bss 77432 77432 0 0.0
.data 1872 1872 0 0.0
.text 576692 576756 64 0.0
linux chip-tool-ipv6only arm64 (read only) 8042100 8051044 8944 0.1
(read/write) 370641 370641 0 0.0
.bss 55217 55217 0 0.0
.data 1096 1096 0 0.0
.data.rel.ro 245856 245856 0 0.0
.dynamic 560 560 0 0.0
.got 64776 64776 0 0.0
.init 24 24 0 0.0
.init_array 168 168 0 0.0
.rodata 419420 419420 0 0.0
.text 6833684 6842628 8944 0.1
thermostat-no-ble arm64 (read only) 2042108 2043020 912 0.0
(read/write) 145969 145969 0 0.0
.bss 65089 65089 0 0.0
.data 880 880 0 0.0
.data.rel.ro 73016 73016 0 0.0
.dynamic 560 560 0 0.0
.got 4048 4048 0 0.0
.init 24 24 0 0.0
.init_array 304 304 0 0.0
.rodata 129884 129884 0 0.0
.text 1697632 1698544 912 0.1
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2349856 2350560 704 0.0
.bss 189220 189220 0 0.0
.data 5320 5320 0 0.0
.text 1312432 1313136 704 0.1
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2334504 2334632 128 0.0
.bss 180760 180760 0 0.0
.data 5568 5568 0 0.0
.text 1297104 1297232 128 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2304848 2304976 128 0.0
.bss 179768 179768 0 0.0
.data 5544 5544 0 0.0
.text 1267448 1267576 128 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1139712 1139712 0 0.0
.bss 11756 11756 0 0.0
.data 4368 4368 0 0.0
.text 103096 103096 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2054464 2054464 0 0.0
.bss 156876 156876 0 0.0
.data 4864 4864 0 0.0
.text 1017064 1017064 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 943403 943499 96 0.0
bss 119628 119628 0 0.0
rodata 108824 108824 0 0.0
text 637376 637468 92 0.0
nrf52840dk_nrf52840+rpc (read/write) 928883 928979 96 0.0
bss 116672 116672 0 0.0
rodata 101272 101272 0 0.0
text 632772 632864 92 0.0
nrf52840dongle_nrf52840 (read/write) 994079 994175 96 0.0
bss 122472 122472 0 0.0
rodata 113576 113576 0 0.0
text 669576 669668 92 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 853242 853322 80 0.0
bss 116416 116416 0 0.0
rodata 101996 101996 0 0.0
text 554300 554392 92 0.0
lock-app nrf52840dk_nrf52840 (read/write) 912763 912827 64 0.0
bss 118784 118784 0 0.0
rodata 103792 103792 0 0.0
text 612824 612880 56 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 822830 822878 48 0.0
bss 115600 115600 0 0.0
rodata 97016 97016 0 0.0
text 529784 529836 52 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 541835 541835 0 0.0
bss 52588 52588 0 0.0
rodata 50104 50104 0 0.0
text 376940 376940 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 915627 915675 48 0.0
bss 118548 118548 0 0.0
rodata 104152 104152 0 0.0
text 615480 615532 52 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 910715 910763 48 0.0
bss 118572 118572 0 0.0
rodata 103264 103264 0 0.0
text 611404 611460 56 0.0
shell nrf52840dk_nrf52840 (read/write) 798655 798655 0 0.0
bss 109776 109776 0 0.0
rodata 78388 78388 0 0.0
text 533992 533992 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 711470 711470 0 0.0
bss 107664 107664 0 0.0
rodata 72688 72688 0 0.0
text 451668 451668 0 0.0
p6 all-clusters-app default (read/write) 2404384 2405216 832 0.0
.bss 117492 117492 0 0.0
.data 2600 2600 0 0.0
.text 1362648 1363480 832 0.1
light-app default (read/write) 2329688 2329752 64 0.0
.bss 106064 106064 0 0.0
.data 2392 2392 0 0.0
.text 1287952 1288016 64 0.0
lock-app default (read/write) 2298712 2298744 32 0.0
.bss 104920 104920 0 0.0
.data 2344 2344 0 0.0
.text 1256976 1257008 32 0.0
qpg lighting-app qpg6105+debug (read only) 563984 564080 96 0.0
(read/write) 146940 146940 0 0.0
.bss 89960 89960 0 0.0
.data 1048 1048 0 0.0
.text 558664 558760 96 0.0
lock-app qpg6105+debug (read only) 515484 515548 64 0.0
(read/write) 146936 146936 0 0.0
.bss 88584 88584 0 0.0
.data 972 972 0 0.0
.text 510164 510228 64 0.0
persistent-storage-app qpg6105+debug (read only) 106848 106848 0 0.0
(read/write) 146940 146940 0 0.0
.bss 38512 38512 0 0.0
.data 288 288 0 0.0
.text 101528 101528 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 840778 840970 192 0.0
bss 87316 87316 0 0.0
noinit 37160 37160 0 0.0
text 587748 587940 192 0.0

Keep the other changes, and adjust the documentation.
@github-actions
Copy link

github-actions bot commented Jan 15, 2022

PR #13607: Size comparison from a11413e to 1ecd2a4

Increases (25 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section a11413e 1ecd2a4 change % change
efr32 lighting-app BRD4161A (read only) 834852 834868 16 0.0
.text 834844 834860 16 0.0
BRD4161A+rpc (read only) 822232 822264 32 0.0
.text 822224 822256 32 0.0
window-app BRD4161A (read only) 805424 805456 32 0.0
.text 805416 805448 32 0.0
esp32 all-clusters-app c3devkit (read only) 912094 912342 248 0.0
.flash.text 912094 912342 248 0.0
m5stack (read only) 960883 961175 292 0.0
.flash.text 955499 955791 292 0.0
k32w lock k32w061+release (read/write) 661796 661812 16 0.0
.text 576692 576708 16 0.0
linux chip-tool-ipv6only arm64 (read only) 8042100 8049492 7392 0.1
.text 6833684 6841076 7392 0.1
thermostat-no-ble arm64 (read only) 2042108 2042812 704 0.0
.text 1697632 1698336 704 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read/write) 2349856 2350112 256 0.0
.text 1312432 1312688 256 0.0
lighting-app CY8CPROTO_062_4343W+release (read/write) 2334504 2334568 64 0.0
.text 1297104 1297168 64 0.0
lock-app CY8CPROTO_062_4343W+release (read/write) 2304848 2304912 64 0.0
.text 1267448 1267512 64 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 943403 943419 16 0.0
text 637376 637388 12 0.0
nrf52840dk_nrf52840+rpc (read/write) 928883 928899 16 0.0
text 632772 632784 12 0.0
nrf52840dongle_nrf52840 (read/write) 994079 994095 16 0.0
text 669576 669588 12 0.0
nrf5340dk_nrf5340_cpuapp text 554300 554312 12 0.0
lock-app nrf52840dk_nrf52840 (read/write) 912763 912779 16 0.0
text 612824 612836 12 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 822830 822846 16 0.0
text 529784 529796 12 0.0
pump-app nrf52840dk_nrf52840 (read/write) 915627 915643 16 0.0
text 615480 615492 12 0.0
pump-controller-app nrf52840dk_nrf52840 text 611404 611416 12 0.0
p6 all-clusters-app default (read/write) 2404384 2404832 448 0.0
.text 1362648 1363096 448 0.0
light-app default (read/write) 2329688 2329704 16 0.0
.text 1287952 1287968 16 0.0
lock-app default (read/write) 2298712 2298728 16 0.0
.text 1256976 1256992 16 0.0
qpg lighting-app qpg6105+debug (read only) 563984 564000 16 0.0
.text 558664 558680 16 0.0
lock-app qpg6105+debug (read only) 515484 515500 16 0.0
.text 510164 510180 16 0.0
telink lighting-app tlsr9518adk80d (read/write) 840778 840826 48 0.0
text 587748 587790 42 0.0
Full report (32 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section a11413e 1ecd2a4 change % change
efr32 lighting-app BRD4161A (read only) 834852 834868 16 0.0
(read/write) 127628 127628 0 0.0
.bss 125744 125744 0 0.0
.data 1884 1884 0 0.0
.text 834844 834860 16 0.0
BRD4161A+rpc (read only) 822232 822264 32 0.0
(read/write) 144288 144288 0 0.0
.bss 142304 142304 0 0.0
.data 1984 1984 0 0.0
.text 822224 822256 32 0.0
window-app BRD4161A (read only) 805424 805456 32 0.0
(read/write) 126320 126320 0 0.0
.bss 124480 124480 0 0.0
.data 1836 1836 0 0.0
.text 805416 805448 32 0.0
esp32 all-clusters-app c3devkit (read only) 912094 912342 248 0.0
(read/write) 1316682 1316682 0 0.0
.dram0.bss 70520 70520 0 0.0
.dram0.data 14284 14284 0 0.0
.flash.rodata 178384 178384 0 0.0
.flash.text 912094 912342 248 0.0
.iram0.text 62056 62056 0 0.0
m5stack (read only) 960883 961175 292 0.0
(read/write) 448840 448840 0 0.0
.dram0.bss 74976 74976 0 0.0
.dram0.data 34064 34064 0 0.0
.flash.rodata 207672 207672 0 0.0
.flash.text 955499 955791 292 0.0
.iram0.text 123399 123399 0 0.0
k32w light k32w061+release (read/write) 657920 657920 0 0.0
.bss 77136 77136 0 0.0
.data 1852 1852 0 0.0
.text 573132 573132 0 0.0
lock k32w061+release (read/write) 661796 661812 16 0.0
.bss 77432 77432 0 0.0
.data 1872 1872 0 0.0
.text 576692 576708 16 0.0
linux chip-tool-ipv6only arm64 (read only) 8042100 8049492 7392 0.1
(read/write) 370641 370641 0 0.0
.bss 55217 55217 0 0.0
.data 1096 1096 0 0.0
.data.rel.ro 245856 245856 0 0.0
.dynamic 560 560 0 0.0
.got 64776 64776 0 0.0
.init 24 24 0 0.0
.init_array 168 168 0 0.0
.rodata 419420 419420 0 0.0
.text 6833684 6841076 7392 0.1
thermostat-no-ble arm64 (read only) 2042108 2042812 704 0.0
(read/write) 145969 145969 0 0.0
.bss 65089 65089 0 0.0
.data 880 880 0 0.0
.data.rel.ro 73016 73016 0 0.0
.dynamic 560 560 0 0.0
.got 4048 4048 0 0.0
.init 24 24 0 0.0
.init_array 304 304 0 0.0
.rodata 129884 129884 0 0.0
.text 1697632 1698336 704 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2349856 2350112 256 0.0
.bss 189220 189220 0 0.0
.data 5320 5320 0 0.0
.text 1312432 1312688 256 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2334504 2334568 64 0.0
.bss 180760 180760 0 0.0
.data 5568 5568 0 0.0
.text 1297104 1297168 64 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2304848 2304912 64 0.0
.bss 179768 179768 0 0.0
.data 5544 5544 0 0.0
.text 1267448 1267512 64 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1139712 1139712 0 0.0
.bss 11756 11756 0 0.0
.data 4368 4368 0 0.0
.text 103096 103096 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2054464 2054464 0 0.0
.bss 156876 156876 0 0.0
.data 4864 4864 0 0.0
.text 1017064 1017064 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 943403 943419 16 0.0
bss 119628 119628 0 0.0
rodata 108824 108824 0 0.0
text 637376 637388 12 0.0
nrf52840dk_nrf52840+rpc (read/write) 928883 928899 16 0.0
bss 116672 116672 0 0.0
rodata 101272 101272 0 0.0
text 632772 632784 12 0.0
nrf52840dongle_nrf52840 (read/write) 994079 994095 16 0.0
bss 122472 122472 0 0.0
rodata 113576 113576 0 0.0
text 669576 669588 12 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 853242 853242 0 0.0
bss 116416 116416 0 0.0
rodata 101996 101996 0 0.0
text 554300 554312 12 0.0
lock-app nrf52840dk_nrf52840 (read/write) 912763 912779 16 0.0
bss 118784 118784 0 0.0
rodata 103792 103792 0 0.0
text 612824 612836 12 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 822830 822846 16 0.0
bss 115600 115600 0 0.0
rodata 97016 97016 0 0.0
text 529784 529796 12 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 541835 541835 0 0.0
bss 52588 52588 0 0.0
rodata 50104 50104 0 0.0
text 376940 376940 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 915627 915643 16 0.0
bss 118548 118548 0 0.0
rodata 104152 104152 0 0.0
text 615480 615492 12 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 910715 910715 0 0.0
bss 118572 118572 0 0.0
rodata 103264 103264 0 0.0
text 611404 611416 12 0.0
shell nrf52840dk_nrf52840 (read/write) 798655 798655 0 0.0
bss 109776 109776 0 0.0
rodata 78388 78388 0 0.0
text 533992 533992 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 711470 711470 0 0.0
bss 107664 107664 0 0.0
rodata 72688 72688 0 0.0
text 451668 451668 0 0.0
p6 all-clusters-app default (read/write) 2404384 2404832 448 0.0
.bss 117492 117492 0 0.0
.data 2600 2600 0 0.0
.text 1362648 1363096 448 0.0
light-app default (read/write) 2329688 2329704 16 0.0
.bss 106064 106064 0 0.0
.data 2392 2392 0 0.0
.text 1287952 1287968 16 0.0
lock-app default (read/write) 2298712 2298728 16 0.0
.bss 104920 104920 0 0.0
.data 2344 2344 0 0.0
.text 1256976 1256992 16 0.0
qpg lighting-app qpg6105+debug (read only) 563984 564000 16 0.0
(read/write) 146940 146940 0 0.0
.bss 89960 89960 0 0.0
.data 1048 1048 0 0.0
.text 558664 558680 16 0.0
lock-app qpg6105+debug (read only) 515484 515500 16 0.0
(read/write) 146936 146936 0 0.0
.bss 88584 88584 0 0.0
.data 972 972 0 0.0
.text 510164 510180 16 0.0
persistent-storage-app qpg6105+debug (read only) 106848 106848 0 0.0
(read/write) 146940 146940 0 0.0
.bss 38512 38512 0 0.0
.data 288 288 0 0.0
.text 101528 101528 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 840778 840826 48 0.0
bss 87316 87316 0 0.0
noinit 37160 37160 0 0.0
text 587748 587790 42 0.0

@mlepage-google mlepage-google changed the title Add support for decoding null valued lists Improve DecodableList when not in list Jan 17, 2022
@andy31415 andy31415 merged commit 8952d1c into project-chip:master Jan 17, 2022
selissia pushed a commit to selissia/connectedhomeip that referenced this pull request Jan 28, 2022
* Add support for decoding null valued lists

Spec treats empty lists and null valued lists equivalently.

However DecodableList expects a container, even if empty, and does
not support the case where there is no container (i.e. null valued
list).

Add support for null valued list (i.e. no container) by handling
it as a special case in the Decode function, the DecodableList,
and its Iterator. This is done using an "empty" mReader (with
container type "not specified") in the "no list" case.

* Remove changes to Decode

Keep the other changes, and adjust the documentation.
step0035 pushed a commit to hank820/connectedhomeip that referenced this pull request Feb 8, 2022
* Add support for decoding null valued lists

Spec treats empty lists and null valued lists equivalently.

However DecodableList expects a container, even if empty, and does
not support the case where there is no container (i.e. null valued
list).

Add support for null valued list (i.e. no container) by handling
it as a special case in the Decode function, the DecodableList,
and its Iterator. This is done using an "empty" mReader (with
container type "not specified") in the "no list" case.

* Remove changes to Decode

Keep the other changes, and adjust the documentation.
@mlepage-google mlepage-google deleted the null-valued-list-decode branch February 15, 2022 15:06
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.

4 participants