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.

Sign up for GitHub

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

Jump to bottom

Refactor read/write ember compatibility functions #12109

Merged
merged 3 commits into from
Dec 3, 2021
Merged

Refactor read/write ember compatibility functions #12109

merged 3 commits into from
Dec 3, 2021

Conversation

mlepage-google
Copy link
Contributor

@mlepage-google mlepage-google commented Nov 22, 2021 •
edited
Loading

Refactor the code within ReadSingleClusterData and
WriteSingleClusterData so it has better support for early exit while
returning status in the message response.

This will be needed soon when Access Control starts denying access.

Tested by running unit tests, and running chip-tool against all-clusters-app
for some reads and writes.

@todo
Copy link

todo bot commented Nov 22, 2021

We should probably clone the writer and convert failures here

connectedhomeip/src/app/util/ember-compatibility-functions.cpp

Lines 243 to 253 in 51f9135

// TODO: We should probably clone the writer and convert failures here
// into status responses, unless our caller already does that.
AttributeValueEncoder valueEncoder(writer, aAccessingFabricIndex);
ReturnErrorOnFailure(attrOverride->Read(readPath, valueEncoder));
if (valueEncoder.TriedEncode())
{
status = Protocols::InteractionModel::Status::Success;
ExitNow();
}
}

This comment was generated by todo based on a TODO comment in 51f9135 in #12109. cc @mlepage-google.

@boring-cyborg boring-cyborg bot added the app label Nov 22, 2021
@github-actions
Copy link

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

PR #12109: Size comparison from 63ea01b to 51f9135

Decreases (15 builds for k32w, linux, p6, qpg, telink)
platform target config section 63ea01b 51f9135b change % change
k32w lighting-app k32w061+se05x+release (read/write) 707544 707464 -80 -0.0
.text 622620 622540 -80 -0.0
lock-app k32w061+debug (read/write) 598976 598896 -80 -0.0
.text 523580 523500 -80 -0.0
shell k32w061+debug (read/write) 664352 664272 -80 -0.0
.text 577828 577748 -80 -0.0
linux all-clusters-app debug (read only) 1746633 1746121 -512 -0.0
.text 1473218 1472706 -512 -0.0
bridge-app debug+rpc (read only) 1326445 1325917 -528 -0.0
.text 1116725 1116197 -528 -0.0
chip-tool debug (read only) 5867333 5866805 -528 -0.0
.text 5210181 5209653 -528 -0.0
lighting-app debug+rpc (read only) 1595553 1595041 -512 -0.0
.text 1330610 1330098 -512 -0.0
ota-provider-app debug (read only) 1273993 1273465 -528 -0.0
.text 1062066 1061538 -528 -0.0
ota-requestor-app debug (read only) 1370241 1369713 -528 -0.0
.text 1144130 1143602 -528 -0.0
tv-app debug (read only) 1888169 1887657 -512 -0.0
.text 1584098 1583586 -512 -0.0
p6 all-clusters-app default (read/write) 2303048 2302936 -112 -0.0
.text 1261312 1261200 -112 -0.0
lock-app default (read/write) 2216248 2216136 -112 -0.0
.text 1174512 1174400 -112 -0.0
qpg lighting-app qpg6100+debug (read only) 494320 494224 -96 -0.0
.text 489000 488904 -96 -0.0
lock-app qpg6100+debug (read only) 469276 469180 -96 -0.0
.text 463956 463860 -96 -0.0
telink lighting-app tlsr9518adk80d (read/write) 768698 768642 -56 -0.0
text 533836 533780 -56 -0.0
Full report (17 builds for k32w, linux, p6, qpg, telink)
platform target config section 63ea01b 51f9135b change % change
k32w lighting-app k32w061+se05x+release (read/write) 707544 707464 -80 -0.0
.bss 77212 77212 0 0.0
.data 1912 1912 0 0.0
.text 622620 622540 -80 -0.0
lock-app k32w061+debug (read/write) 598976 598896 -80 -0.0
.bss 67716 67716 0 0.0
.data 1880 1880 0 0.0
.text 523580 523500 -80 -0.0
shell k32w061+debug (read/write) 664352 664272 -80 -0.0
.bss 78876 78876 0 0.0
.data 1848 1848 0 0.0
.text 577828 577748 -80 -0.0
linux all-clusters-app debug (read only) 1746633 1746121 -512 -0.0
(read/write) 128392 128392 0 0.0
.bss 58544 58544 0 0.0
.data 1042 1042 0 0.0
.data.rel.ro 63504 63504 0 0.0
.dynamic 592 592 0 0.0
.got 4112 4112 0 0.0
.init 27 27 0 0.0
.init_array 560 560 0 0.0
.rodata 137781 137781 0 0.0
.text 1473218 1472706 -512 -0.0
bridge-app debug+rpc (read only) 1326445 1325917 -528 -0.0
(read/write) 76432 76432 0 0.0
.bss 41520 41520 0 0.0
.data 1568 1568 0 0.0
.data.rel.ro 28352 28352 0 0.0
.dynamic 592 592 0 0.0
.got 3984 3984 0 0.0
.init 27 27 0 0.0
.init_array 408 408 0 0.0
.rodata 112316 112316 0 0.0
.text 1116725 1116197 -528 -0.0
chip-tool debug (read only) 5867333 5866805 -528 -0.0
(read/write) 195496 195496 0 0.0
.bss 39848 39848 0 0.0
.data 2272 2272 0 0.0
.data.rel.ro 147872 147872 0 0.0
.dynamic 592 592 0 0.0
.got 4416 4416 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 278122 278122 0 0.0
.text 5210181 5209653 -528 -0.0
lighting-app debug+rpc (read only) 1595553 1595041 -512 -0.0
(read/write) 109616 109616 0 0.0
.bss 47152 47152 0 0.0
.data 1234 1234 0 0.0
.data.rel.ro 55920 55920 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 536 536 0 0.0
.rodata 129937 129937 0 0.0
.text 1330610 1330098 -512 -0.0
ota-provider-app debug (read only) 1273993 1273465 -528 -0.0
(read/write) 75048 75048 0 0.0
.bss 44096 44096 0 0.0
.data 784 784 0 0.0
.data.rel.ro 25080 25080 0 0.0
.dynamic 592 592 0 0.0
.got 4016 4016 0 0.0
.init 27 27 0 0.0
.init_array 448 448 0 0.0
.rodata 114335 114335 0 0.0
.text 1062066 1061538 -528 -0.0
ota-requestor-app debug (read only) 1370241 1369713 -528 -0.0
(read/write) 78912 78912 0 0.0
.bss 46560 46560 0 0.0
.data 848 848 0 0.0
.data.rel.ro 26408 26408 0 0.0
.dynamic 592 592 0 0.0
.got 4000 4000 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 125736 125736 0 0.0
.text 1144130 1143602 -528 -0.0
shell debug (read only) 799185 799185 0 0.0
(read/write) 65832 65832 0 0.0
.bss 23336 23336 0 0.0
.data 242 242 0 0.0
.data.rel.ro 37752 37752 0 0.0
.dynamic 592 592 0 0.0
.got 3528 3528 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 78703 78703 0 0.0
.text 616466 616466 0 0.0
tv-app debug (read only) 1888169 1887657 -512 -0.0
(read/write) 318520 318520 0 0.0
.bss 249800 249800 0 0.0
.data 2768 2768 0 0.0
.data.rel.ro 60264 60264 0 0.0
.dynamic 592 592 0 0.0
.got 4432 4432 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 158629 158629 0 0.0
.text 1584098 1583586 -512 -0.0
p6 all-clusters-app default (read/write) 2303048 2302936 -112 -0.0
.bss 113280 113280 0 0.0
.data 2528 2528 0 0.0
.heap 917536 917536 0 0.0
.text 1261312 1261200 -112 -0.0
lock-app default (read/write) 2216248 2216136 -112 -0.0
.bss 100944 100944 0 0.0
.data 2400 2400 0 0.0
.heap 930000 930000 0 0.0
.text 1174512 1174400 -112 -0.0
qpg lighting-app qpg6100+debug (read only) 494320 494224 -96 -0.0
(read/write) 114144 114144 0 0.0
.bss 50256 50256 0 0.0
.data 1008 1008 0 0.0
.text 489000 488904 -96 -0.0
lock-app qpg6100+debug (read only) 469276 469180 -96 -0.0
(read/write) 114140 114140 0 0.0
.bss 49200 49200 0 0.0
.data 964 964 0 0.0
.text 463956 463860 -96 -0.0
persistent-storage-app qpg6100+debug (read only) 105416 105416 0 0.0
(read/write) 114142 114142 0 0.0
.bss 8994 8994 0 0.0
.data 272 272 0 0.0
.text 100096 100096 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 768698 768642 -56 -0.0
bss 79116 79116 0 0.0
noinit 37160 37160 0 0.0
text 533836 533780 -56 -0.0

andy31415
andy31415 reviewed Nov 22, 2021
View reviewed changes
src/app/util/ember-compatibility-functions.cpp Outdated Show resolved Hide resolved
src/app/util/ember-compatibility-functions.cpp Outdated Show resolved Hide resolved
src/app/util/ember-compatibility-functions.cpp Outdated Show resolved Hide resolved
@github-actions
Copy link

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

PR #12109: Size comparison from 3f2ab62 to f0c0836

Decreases (31 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 3f2ab62 f0c0836 change % change
efr32 lighting-app BRD4161A (read only) 750776 750664 -112 -0.0
.text 750768 750656 -112 -0.0
BRD4161A+rpc (read only) 738308 738196 -112 -0.0
.text 738300 738188 -112 -0.0
lock-app BRD4161A (read only) 727848 727736 -112 -0.0
.text 727840 727728 -112 -0.0
window-app BRD4161A (read only) 731296 731184 -112 -0.0
.text 731288 731176 -112 -0.0
esp32 all-clusters-app c3devkit (read only) 833422 833358 -64 -0.0
.flash.text 833422 833358 -64 -0.0
m5stack (read only) 904443 904315 -128 -0.0
.flash.text 899059 898931 -128 -0.0
k32w lighting-app k32w061+se05x+release (read/write) 707564 707484 -80 -0.0
.text 622640 622560 -80 -0.0
lock-app k32w061+debug (read/write) 599004 598924 -80 -0.0
.text 523600 523520 -80 -0.0
shell k32w061+debug (read/write) 664372 664292 -80 -0.0
.text 577848 577768 -80 -0.0
linux all-clusters-app debug (read only) 1746657 1746145 -512 -0.0
.text 1473218 1472706 -512 -0.0
bridge-app debug+rpc (read only) 1326469 1325941 -528 -0.0
.text 1116725 1116197 -528 -0.0
chip-tool debug (read only) 5868429 5867917 -512 -0.0
.text 5211109 5210597 -512 -0.0
lighting-app debug+rpc (read only) 1595577 1595065 -512 -0.0
.text 1330610 1330098 -512 -0.0
ota-provider-app debug (read only) 1273993 1273465 -528 -0.0
.text 1062066 1061538 -528 -0.0
ota-requestor-app debug (read only) 1370241 1369713 -528 -0.0
.text 1144130 1143602 -528 -0.0
tv-app debug (read only) 1888193 1887681 -512 -0.0
.text 1584098 1583586 -512 -0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read/write) 2290280 2290216 -64 -0.0
.text 1252880 1252816 -64 -0.0
lighting-app CY8CPROTO_062_4343W+release (read/write) 2274408 2274344 -64 -0.0
.text 1237008 1236944 -64 -0.0
lock-app CY8CPROTO_062_4343W+release (read/write) 2250240 2250176 -64 -0.0
.text 1212840 1212776 -64 -0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 865287 865223 -64 -0.0
text 583212 583136 -76 -0.0
nrf52840dk_nrf52840+rpc (read/write) 827655 827591 -64 -0.0
text 557380 557304 -76 -0.0
nrf5340dk_nrf5340_cpuapp (read/write) 790342 790262 -80 -0.0
text 512684 512608 -76 -0.0
lock-app nrf52840dk_nrf52840 (read/write) 840687 840607 -80 -0.0
text 563308 563232 -76 -0.0
nrf5340dk_nrf5340_cpuapp (read/write) 766022 765942 -80 -0.0
text 492868 492792 -76 -0.0
pump-app nrf52840dk_nrf52840 (read/write) 846827 846747 -80 -0.0
text 567488 567412 -76 -0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 840419 840339 -80 -0.0
text 562928 562852 -76 -0.0
p6 all-clusters-app default (read/write) 2303072 2302960 -112 -0.0
.text 1261336 1261224 -112 -0.0
lock-app default (read/write) 2216272 2216160 -112 -0.0
.text 1174536 1174424 -112 -0.0
qpg lighting-app qpg6100+debug (read only) 494340 494244 -96 -0.0
.text 489020 488924 -96 -0.0
lock-app qpg6100+debug (read only) 469296 469200 -96 -0.0
.text 463976 463880 -96 -0.0
telink lighting-app tlsr9518adk80d (read/write) 768718 768662 -56 -0.0
text 533836 533780 -56 -0.0
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 3f2ab62 f0c0836 change % change
efr32 lighting-app BRD4161A (read only) 750776 750664 -112 -0.0
(read/write) 119692 119692 0 0.0
.bss 117892 117892 0 0.0
.data 1800 1800 0 0.0
.text 750768 750656 -112 -0.0
BRD4161A+rpc (read only) 738308 738196 -112 -0.0
(read/write) 136320 136320 0 0.0
.bss 134396 134396 0 0.0
.data 1924 1924 0 0.0
.text 738300 738188 -112 -0.0
lock-app BRD4161A (read only) 727848 727736 -112 -0.0
(read/write) 117476 117476 0 0.0
.bss 115716 115716 0 0.0
.data 1756 1756 0 0.0
.text 727840 727728 -112 -0.0
window-app BRD4161A (read only) 731296 731184 -112 -0.0
(read/write) 117836 117836 0 0.0
.bss 116068 116068 0 0.0
.data 1764 1764 0 0.0
.text 731288 731176 -112 -0.0
esp32 all-clusters-app c3devkit (read only) 833422 833358 -64 -0.0
(read/write) 1221962 1221962 0 0.0
.dram0.bss 57760 57760 0 0.0
.dram0.data 14092 14092 0 0.0
.flash.rodata 164864 164864 0 0.0
.flash.text 833422 833358 -64 -0.0
.iram0.text 61394 61394 0 0.0
m5stack (read only) 904443 904315 -128 -0.0
(read/write) 421256 421256 0 0.0
.dram0.bss 63112 63112 0 0.0
.dram0.data 34056 34056 0 0.0
.flash.rodata 192808 192808 0 0.0
.flash.text 899059 898931 -128 -0.0
.iram0.text 122943 122943 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 707564 707484 -80 -0.0
.bss 77212 77212 0 0.0
.data 1912 1912 0 0.0
.text 622640 622560 -80 -0.0
lock-app k32w061+debug (read/write) 599004 598924 -80 -0.0
.bss 67724 67724 0 0.0
.data 1880 1880 0 0.0
.text 523600 523520 -80 -0.0
shell k32w061+debug (read/write) 664372 664292 -80 -0.0
.bss 78876 78876 0 0.0
.data 1848 1848 0 0.0
.text 577848 577768 -80 -0.0
linux all-clusters-app debug (read only) 1746657 1746145 -512 -0.0
(read/write) 128392 128392 0 0.0
.bss 58544 58544 0 0.0
.data 1042 1042 0 0.0
.data.rel.ro 63504 63504 0 0.0
.dynamic 592 592 0 0.0
.got 4112 4112 0 0.0
.init 27 27 0 0.0
.init_array 560 560 0 0.0
.rodata 137781 137781 0 0.0
.text 1473218 1472706 -512 -0.0
bridge-app debug+rpc (read only) 1326469 1325941 -528 -0.0
(read/write) 76432 76432 0 0.0
.bss 41520 41520 0 0.0
.data 1568 1568 0 0.0
.data.rel.ro 28352 28352 0 0.0
.dynamic 592 592 0 0.0
.got 3984 3984 0 0.0
.init 27 27 0 0.0
.init_array 408 408 0 0.0
.rodata 112316 112316 0 0.0
.text 1116725 1116197 -528 -0.0
chip-tool debug (read only) 5868429 5867917 -512 -0.0
(read/write) 195560 195560 0 0.0
.bss 39848 39848 0 0.0
.data 2272 2272 0 0.0
.data.rel.ro 147936 147936 0 0.0
.dynamic 592 592 0 0.0
.got 4416 4416 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 278122 278122 0 0.0
.text 5211109 5210597 -512 -0.0
lighting-app debug+rpc (read only) 1595577 1595065 -512 -0.0
(read/write) 109680 109680 0 0.0
.bss 47184 47184 0 0.0
.data 1234 1234 0 0.0
.data.rel.ro 55952 55952 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 536 536 0 0.0
.rodata 129937 129937 0 0.0
.text 1330610 1330098 -512 -0.0
ota-provider-app debug (read only) 1273993 1273465 -528 -0.0
(read/write) 75048 75048 0 0.0
.bss 44096 44096 0 0.0
.data 784 784 0 0.0
.data.rel.ro 25080 25080 0 0.0
.dynamic 592 592 0 0.0
.got 4016 4016 0 0.0
.init 27 27 0 0.0
.init_array 448 448 0 0.0
.rodata 114335 114335 0 0.0
.text 1062066 1061538 -528 -0.0
ota-requestor-app debug (read only) 1370241 1369713 -528 -0.0
(read/write) 78912 78912 0 0.0
.bss 46560 46560 0 0.0
.data 848 848 0 0.0
.data.rel.ro 26408 26408 0 0.0
.dynamic 592 592 0 0.0
.got 4000 4000 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 125736 125736 0 0.0
.text 1144130 1143602 -528 -0.0
shell debug (read only) 799185 799185 0 0.0
(read/write) 65832 65832 0 0.0
.bss 23336 23336 0 0.0
.data 242 242 0 0.0
.data.rel.ro 37752 37752 0 0.0
.dynamic 592 592 0 0.0
.got 3528 3528 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 78703 78703 0 0.0
.text 616466 616466 0 0.0
tv-app debug (read only) 1888193 1887681 -512 -0.0
(read/write) 318520 318520 0 0.0
.bss 249800 249800 0 0.0
.data 2768 2768 0 0.0
.data.rel.ro 60264 60264 0 0.0
.dynamic 592 592 0 0.0
.got 4432 4432 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 158629 158629 0 0.0
.text 1584098 1583586 -512 -0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2290280 2290216 -64 -0.0
.bss 180316 180316 0 0.0
.data 5224 5224 0 0.0
.heap 850904 850904 0 0.0
.text 1252880 1252816 -64 -0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2274408 2274344 -64 -0.0
.bss 172212 172212 0 0.0
.data 5576 5576 0 0.0
.heap 858656 858656 0 0.0
.text 1237008 1236944 -64 -0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2250240 2250176 -64 -0.0
.bss 171092 171092 0 0.0
.data 5568 5568 0 0.0
.heap 859784 859784 0 0.0
.text 1212840 1212776 -64 -0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1139744 1139744 0 0.0
.bss 11752 11752 0 0.0
.data 4368 4368 0 0.0
.heap 1020328 1020328 0 0.0
.text 103128 103128 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2050544 2050544 0 0.0
.bss 156280 156280 0 0.0
.data 4968 4968 0 0.0
.heap 875200 875200 0 0.0
.text 1013144 1013144 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 865287 865223 -64 -0.0
bss 110608 110608 0 0.0
rodata 95848 95848 0 0.0
text 583212 583136 -76 -0.0
nrf52840dk_nrf52840+rpc (read/write) 827655 827591 -64 -0.0
bss 106964 106964 0 0.0
rodata 87032 87032 0 0.0
text 557380 557304 -76 -0.0
nrf5340dk_nrf5340_cpuapp (read/write) 790342 790262 -80 -0.0
bss 111984 111984 0 0.0
rodata 91104 91104 0 0.0
text 512684 512608 -76 -0.0
lock-app nrf52840dk_nrf52840 (read/write) 840687 840607 -80 -0.0
bss 109632 109632 0 0.0
rodata 92320 92320 0 0.0
text 563308 563232 -76 -0.0
nrf5340dk_nrf5340_cpuapp (read/write) 766022 765942 -80 -0.0
bss 111044 111044 0 0.0
rodata 87624 87624 0 0.0
text 492868 492792 -76 -0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 497327 497327 0 0.0
bss 51824 51824 0 0.0
rodata 45780 45780 0 0.0
text 339436 339436 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 846827 846747 -80 -0.0
bss 109772 109772 0 0.0
rodata 94028 94028 0 0.0
text 567488 567412 -76 -0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 840419 840339 -80 -0.0
bss 109668 109668 0 0.0
rodata 92276 92276 0 0.0
text 562928 562852 -76 -0.0
shell nrf52840dk_nrf52840 (read/write) 778003 778003 0 0.0
bss 109112 109112 0 0.0
rodata 73064 73064 0 0.0
text 521244 521244 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 693058 693058 0 0.0
bss 110096 110096 0 0.0
rodata 67708 67708 0 0.0
text 441856 441856 0 0.0
p6 all-clusters-app default (read/write) 2303072 2302960 -112 -0.0
.bss 113288 113288 0 0.0
.data 2528 2528 0 0.0
.heap 917528 917528 0 0.0
.text 1261336 1261224 -112 -0.0
lock-app default (read/write) 2216272 2216160 -112 -0.0
.bss 100944 100944 0 0.0
.data 2400 2400 0 0.0
.heap 930000 930000 0 0.0
.text 1174536 1174424 -112 -0.0
qpg lighting-app qpg6100+debug (read only) 494340 494244 -96 -0.0
(read/write) 114144 114144 0 0.0
.bss 50264 50264 0 0.0
.data 1008 1008 0 0.0
.text 489020 488924 -96 -0.0
lock-app qpg6100+debug (read only) 469296 469200 -96 -0.0
(read/write) 114140 114140 0 0.0
.bss 49200 49200 0 0.0
.data 964 964 0 0.0
.text 463976 463880 -96 -0.0
persistent-storage-app qpg6100+debug (read only) 105416 105416 0 0.0
(read/write) 114142 114142 0 0.0
.bss 8994 8994 0 0.0
.data 272 272 0 0.0
.text 100096 100096 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 768718 768662 -56 -0.0
bss 79120 79120 0 0.0
noinit 37160 37160 0 0.0
text 533836 533780 -56 -0.0

yunhanw-google
yunhanw-google approved these changes Nov 23, 2021
View reviewed changes
Copy link
Contributor

@yunhanw-google yunhanw-google left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

src/app/util/ember-compatibility-functions.cpp Outdated Show resolved Hide resolved
@woody-apple
Copy link
Contributor

Fast tracking given this has been in review for a while.

@mlepage-google
Copy link
Contributor Author

I'm fixing this PR now so it can be merged.

bzbarsky-apple
bzbarsky-apple requested changes Dec 2, 2021
View reviewed changes
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

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

Need to merge to tip and fix the existing merge issues...

src/app/util/ember-compatibility-functions.cpp Outdated Show resolved Hide resolved
src/app/util/ember-compatibility-functions.cpp Outdated Show resolved Hide resolved
src/app/util/ember-compatibility-functions.cpp Outdated Show resolved Hide resolved
src/app/util/ember-compatibility-functions.cpp Outdated Show resolved Hide resolved
@mlepage-google
Redo refactoring on top of latest master branch
ae5c82f 
Also improved the refactoring, it should be clearer.
@mlepage-google mlepage-google force-pushed the refactor-ember-read-write branch from f0c0836 to ae5c82f Compare December 3, 2021 00:41
@todo
Copy link

todo bot commented Dec 3, 2021

Refactor WriteSingleClusterData and all dependent functions to take ConcreteAttributePath instead of ClusterInfo

connectedhomeip/src/app/util/ember-compatibility-functions.cpp

Lines 663 to 673 in ae5c82f

// TODO: Refactor WriteSingleClusterData and all dependent functions to take ConcreteAttributePath instead of ClusterInfo
// as the input argument.
CHIP_ERROR WriteSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVReader & aReader, WriteHandler * apWriteHandler)
{
// Named aPath for now to reduce the amount of code change that needs to
// happen when the above TODO is resolved.
ConcreteDataAttributePath aPath(aClusterInfo.mEndpointId, aClusterInfo.mClusterId, aClusterInfo.mAttributeId);
const EmberAfAttributeMetadata * attributeMetadata =
emberAfLocateAttributeMetadata(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId, CLUSTER_MASK_SERVER, 0);
AttributePathParams attributePathParams = { .mEndpointId = aPath.mEndpointId,

This comment was generated by todo based on a TODO comment in ae5c82f in #12109. cc @mlepage-google.

@mlepage-google
Copy link
Contributor Author

I redid the changes on latest master branch. Also, I simplified how the refactoring was done, so it should be clearer now.

@mlepage-google
Fix initialization of AttributePathParams
fdf2f26 
Apparently it doesn't like that syntax.
@github-actions
Copy link

github-actions bot commented Dec 3, 2021 •
edited
Loading

PR #12109: Size comparison from 1ad526b to 76f71eb

Decreases (32 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 1ad526b 76f71eb change % change
efr32 lighting-app BRD4161A (read only) 751584 751520 -64 -0.0
.text 751576 751512 -64 -0.0
BRD4161A+rpc (read only) 780256 780192 -64 -0.0
.text 780248 780184 -64 -0.0
lock-app BRD4161A (read only) 725440 725376 -64 -0.0
.text 725432 725368 -64 -0.0
window-app BRD4161A (read only) 728752 728688 -64 -0.0
.text 728744 728680 -64 -0.0
esp32 all-clusters-app c3devkit (read only) 847862 847818 -44 -0.0
.flash.text 847862 847818 -44 -0.0
m5stack (read only) 918439 918215 -224 -0.0
.flash.text 913055 912831 -224 -0.0
k32w lighting-app k32w061+se05x+release (read/write) 728752 728688 -64 -0.0
.text 641788 641724 -64 -0.0
lock-app k32w061+debug (read/write) 617736 617672 -64 -0.0
.text 540368 540304 -64 -0.0
shell k32w061+debug (read/write) 683636 683588 -48 -0.0
.text 594640 594592 -48 -0.0
linux all-clusters-app debug (read only) 1843857 1843505 -352 -0.0
.text 1554882 1554530 -352 -0.0
bridge-app debug+rpc (read only) 1424181 1423845 -336 -0.0
.text 1199957 1199621 -336 -0.0
chip-tool debug (read only) 6619357 6618877 -480 -0.0
.text 5901765 5901285 -480 -0.0
lighting-app debug+rpc (read only) 1708281 1707929 -352 -0.0
.text 1426530 1426178 -352 -0.0
ota-provider-app debug (read only) 1384897 1384561 -336 -0.0
.text 1159618 1159282 -336 -0.0
ota-requestor-app debug (read only) 1488721 1488369 -352 -0.0
.text 1247106 1246754 -352 -0.0
tv-app debug (read only) 2001625 2001273 -352 -0.0
.text 1679250 1678898 -352 -0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read/write) 2309264 2309200 -64 -0.0
.text 1271840 1271776 -64 -0.0
lighting-app CY8CPROTO_062_4343W+release (read/write) 2295656 2295592 -64 -0.0
.text 1258256 1258192 -64 -0.0
lock-app CY8CPROTO_062_4343W+release (read/write) 2268688 2268624 -64 -0.0
.text 1231288 1231224 -64 -0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 891503 891455 -48 -0.0
text 602792 602740 -52 -0.0
nrf52840dk_nrf52840+rpc (read/write) 854879 854831 -48 -0.0
text 577788 577736 -52 -0.0
nrf5340dk_nrf5340_cpuapp (read/write) 817414 817350 -64 -0.0
text 533128 533076 -52 -0.0
lock-app nrf52840dk_nrf52840 (read/write) 862359 862311 -48 -0.0
text 581044 580992 -52 -0.0
nrf5340dk_nrf5340_cpuapp (read/write) 788514 788466 -48 -0.0
text 511472 511420 -52 -0.0
pump-app nrf52840dk_nrf52840 (read/write) 868375 868327 -48 -0.0
text 585108 585056 -52 -0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 861439 861391 -48 -0.0
text 580168 580116 -52 -0.0
p6 all-clusters-app default (read/write) 2339544 2339480 -64 -0.0
.text 1297808 1297744 -64 -0.0
light-app default (read/write) 2278920 2278840 -80 -0.0
.text 1237184 1237104 -80 -0.0
lock-app default (read/write) 2254256 2254192 -64 -0.0
.text 1212520 1212456 -64 -0.0
qpg lighting-app qpg6100+debug (read only) 510544 510480 -64 -0.0
.text 505224 505160 -64 -0.0
lock-app qpg6100+debug (read only) 483352 483288 -64 -0.0
.text 478032 477968 -64 -0.0
telink lighting-app tlsr9518adk80d (read/write) 794594 794546 -48 -0.0
text 554576 554526 -50 -0.0
Full report (39 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 1ad526b 76f71eb change % change
efr32 lighting-app BRD4161A (read only) 751584 751520 -64 -0.0
(read/write) 120032 120032 0 0.0
.bss 118200 118200 0 0.0
.data 1828 1828 0 0.0
.text 751576 751512 -64 -0.0
BRD4161A+rpc (read only) 780256 780192 -64 -0.0
(read/write) 138336 138336 0 0.0
.bss 136400 136400 0 0.0
.data 1936 1936 0 0.0
.text 780248 780184 -64 -0.0
lock-app BRD4161A (read only) 725440 725376 -64 -0.0
(read/write) 117736 117736 0 0.0
.bss 115952 115952 0 0.0
.data 1784 1784 0 0.0
.text 725432 725368 -64 -0.0
window-app BRD4161A (read only) 728752 728688 -64 -0.0
(read/write) 118160 118160 0 0.0
.bss 116368 116368 0 0.0
.data 1792 1792 0 0.0
.text 728744 728680 -64 -0.0
esp32 all-clusters-app c3devkit (read only) 847862 847818 -44 -0.0
(read/write) 1223826 1223826 0 0.0
.dram0.bss 56576 56576 0 0.0
.dram0.data 14052 14052 0 0.0
.flash.rodata 167744 167744 0 0.0
.flash.text 847862 847818 -44 -0.0
.iram0.text 61394 61394 0 0.0
m5stack (read only) 918439 918215 -224 -0.0
(read/write) 422804 422804 0 0.0
.dram0.bss 61960 61960 0 0.0
.dram0.data 34016 34016 0 0.0
.flash.rodata 195548 195548 0 0.0
.flash.text 913055 912831 -224 -0.0
.iram0.text 122943 122943 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 728752 728688 -64 -0.0
.bss 79304 79304 0 0.0
.data 1860 1860 0 0.0
.text 641788 641724 -64 -0.0
lock-app k32w061+debug (read/write) 617736 617672 -64 -0.0
.bss 69744 69744 0 0.0
.data 1824 1824 0 0.0
.text 540368 540304 -64 -0.0
shell k32w061+debug (read/write) 683636 683588 -48 -0.0
.bss 81400 81400 0 0.0
.data 1796 1796 0 0.0
.text 594640 594592 -48 -0.0
linux all-clusters-app debug (read only) 1843857 1843505 -352 -0.0
(read/write) 124256 124256 0 0.0
.bss 50768 50768 0 0.0
.data 1120 1120 0 0.0
.data.rel.ro 67088 67088 0 0.0
.dynamic 592 592 0 0.0
.got 4120 4120 0 0.0
.init 27 27 0 0.0
.init_array 552 552 0 0.0
.rodata 146517 146517 0 0.0
.text 1554882 1554530 -352 -0.0
bridge-app debug+rpc (read only) 1424181 1423845 -336 -0.0
(read/write) 74416 74416 0 0.0
.bss 36144 36144 0 0.0
.data 1728 1728 0 0.0
.data.rel.ro 31528 31528 0 0.0
.dynamic 592 592 0 0.0
.got 3992 3992 0 0.0
.init 27 27 0 0.0
.init_array 408 408 0 0.0
.rodata 119564 119564 0 0.0
.text 1199957 1199621 -336 -0.0
chip-tool debug (read only) 6619357 6618877 -480 -0.0
(read/write) 202128 202128 0 0.0
.bss 34440 34440 0 0.0
.data 1024 1024 0 0.0
.data.rel.ro 161112 161112 0 0.0
.dynamic 592 592 0 0.0
.got 4472 4472 0 0.0
.init 27 27 0 0.0
.init_array 480 480 0 0.0
.rodata 307288 307288 0 0.0
.text 5901765 5901285 -480 -0.0
lighting-app debug+rpc (read only) 1708281 1707929 -352 -0.0
(read/write) 107408 107408 0 0.0
.bss 41808 41808 0 0.0
.data 1280 1280 0 0.0
.data.rel.ro 59024 59024 0 0.0
.dynamic 608 608 0 0.0
.got 4144 4144 0 0.0
.init 27 27 0 0.0
.init_array 536 536 0 0.0
.rodata 139345 139345 0 0.0
.text 1426530 1426178 -352 -0.0
ota-provider-app debug (read only) 1384897 1384561 -336 -0.0
(read/write) 72648 72648 0 0.0
.bss 38720 38720 0 0.0
.data 928 928 0 0.0
.data.rel.ro 27880 27880 0 0.0
.dynamic 592 592 0 0.0
.got 4056 4056 0 0.0
.init 27 27 0 0.0
.init_array 448 448 0 0.0
.rodata 120808 120808 0 0.0
.text 1159618 1159282 -336 -0.0
ota-requestor-app debug (read only) 1488721 1488369 -352 -0.0
(read/write) 76584 76584 0 0.0
.bss 40832 40832 0 0.0
.data 992 992 0 0.0
.data.rel.ro 29592 29592 0 0.0
.dynamic 592 592 0 0.0
.got 4072 4072 0 0.0
.init 27 27 0 0.0
.init_array 480 480 0 0.0
.rodata 133136 133136 0 0.0
.text 1247106 1246754 -352 -0.0
shell debug (read only) 819625 819625 0 0.0
(read/write) 60584 60584 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 3512 3512 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 83570 83570 0 0.0
.text 629314 629314 0 0.0
tv-app debug (read only) 2001625 2001273 -352 -0.0
(read/write) 317648 317648 0 0.0
.bss 246712 246712 0 0.0
.data 1520 1520 0 0.0
.data.rel.ro 63720 63720 0 0.0
.dynamic 592 592 0 0.0
.got 4432 4432 0 0.0
.init 27 27 0 0.0
.init_array 624 624 0 0.0
.rodata 169384 169384 0 0.0
.text 1679250 1678898 -352 -0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2309264 2309200 -64 -0.0
.bss 179628 179628 0 0.0
.data 5184 5184 0 0.0
.heap 851632 851632 0 0.0
.text 1271840 1271776 -64 -0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2295656 2295592 -64 -0.0
.bss 173272 173272 0 0.0
.data 5496 5496 0 0.0
.heap 857680 857680 0 0.0
.text 1258256 1258192 -64 -0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2268688 2268624 -64 -0.0
.bss 172088 172088 0 0.0
.data 5496 5496 0 0.0
.heap 858864 858864 0 0.0
.text 1231288 1231224 -64 -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) 2046960 2046960 0 0.0
.bss 156732 156732 0 0.0
.data 4872 4872 0 0.0
.heap 874840 874840 0 0.0
.text 1009560 1009560 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 891503 891455 -48 -0.0
bss 113752 113752 0 0.0
rodata 99408 99408 0 0.0
text 602792 602740 -52 -0.0
nrf52840dk_nrf52840+rpc (read/write) 854879 854831 -48 -0.0
bss 110100 110100 0 0.0
rodata 90768 90768 0 0.0
text 577788 577736 -52 -0.0
nrf5340dk_nrf5340_cpuapp (read/write) 817414 817350 -64 -0.0
bss 115124 115124 0 0.0
rodata 94664 94664 0 0.0
text 533128 533076 -52 -0.0
lock-app nrf52840dk_nrf52840 (read/write) 862359 862311 -48 -0.0
bss 110788 110788 0 0.0
rodata 95144 95144 0 0.0
text 581044 580992 -52 -0.0
nrf5340dk_nrf5340_cpuapp (read/write) 788514 788466 -48 -0.0
bss 112200 112200 0 0.0
rodata 90436 90436 0 0.0
text 511472 511420 -52 -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) 868375 868327 -48 -0.0
bss 110924 110924 0 0.0
rodata 96880 96880 0 0.0
text 585108 585056 -52 -0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 861439 861391 -48 -0.0
bss 110804 110804 0 0.0
rodata 95016 95016 0 0.0
text 580168 580116 -52 -0.0
shell nrf52840dk_nrf52840 (read/write) 779891 779891 0 0.0
bss 109696 109696 0 0.0
rodata 73792 73792 0 0.0
text 521908 521908 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 694934 694934 0 0.0
bss 110680 110680 0 0.0
rodata 68432 68432 0 0.0
text 442512 442512 0 0.0
p6 all-clusters-app default (read/write) 2339544 2339480 -64 -0.0
.bss 107820 107820 0 0.0
.data 2456 2456 0 0.0
.heap 923064 923064 0 0.0
.text 1297808 1297744 -64 -0.0
light-app default (read/write) 2278920 2278840 -80 -0.0
.bss 98504 98504 0 0.0
.data 2336 2336 0 0.0
.heap 932504 932504 0 0.0
.text 1237184 1237104 -80 -0.0
lock-app default (read/write) 2254256 2254192 -64 -0.0
.bss 97160 97160 0 0.0
.data 2296 2296 0 0.0
.heap 933888 933888 0 0.0
.text 1212520 1212456 -64 -0.0
qpg lighting-app qpg6100+debug (read only) 510544 510480 -64 -0.0
(read/write) 122332 122332 0 0.0
.bss 80264 80264 0 0.0
.data 964 964 0 0.0
.text 505224 505160 -64 -0.0
lock-app qpg6100+debug (read only) 483352 483288 -64 -0.0
(read/write) 122332 122332 0 0.0
.bss 79176 79176 0 0.0
.data 916 916 0 0.0
.text 478032 477968 -64 -0.0
persistent-storage-app qpg6100+debug (read only) 108208 108208 0 0.0
(read/write) 122332 122332 0 0.0
.bss 36696 36696 0 0.0
.data 292 292 0 0.0
.text 102888 102888 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 794594 794546 -48 -0.0
bss 80328 80328 0 0.0
noinit 37160 37160 0 0.0
text 554576 554526 -50 -0.0

@mlepage-google mlepage-google merged commit c8e8696 into project-chip:master Dec 3, 2021
@todo todo bot mentioned this pull request Dec 3, 2021
Closed
@mlepage-google mlepage-google deleted the refactor-ember-read-write branch December 3, 2021 02:26
billwatersiii pushed a commit to billwatersiii/connectedhomeip that referenced this pull request Dec 3, 2021
@mlepage-google @billwatersiii
Refactor read/write ember compatibility functions (project-chip#12109)
b3d88bc 
Refactor the ember read/write functions so they better return a status message
Co-authored-by: Boris Zbarsky <[email protected]>
@mlepage-google mlepage-google mentioned this pull request Dec 14, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yunhanw-google yunhanw-google yunhanw-google approved these changes

@andy31415 andy31415 andy31415 left review comments

@bzbarsky-apple bzbarsky-apple bzbarsky-apple approved these changes

@woody-apple woody-apple woody-apple approved these changes

@anush-apple anush-apple Awaiting requested review from anush-apple

@austinh0 austinh0 Awaiting requested review from austinh0

@balducci-apple balducci-apple Awaiting requested review from balducci-apple

@carol-apple carol-apple Awaiting requested review from carol-apple

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chulspro chulspro Awaiting requested review from chulspro

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@franck-apple franck-apple Awaiting requested review from franck-apple

@harimau-qirex harimau-qirex Awaiting requested review from harimau-qirex

@hawk248 hawk248 Awaiting requested review from hawk248

@holbrookt holbrookt Awaiting requested review from holbrookt

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@LuDuda LuDuda Awaiting requested review from LuDuda

@lzgrablic02 lzgrablic02 Awaiting requested review from lzgrablic02

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@pan-apple pan-apple Awaiting requested review from pan-apple

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tecimovic tecimovic Awaiting requested review from tecimovic

@turon turon Awaiting requested review from turon

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

Assignees
No one assigned
Labels
app fast track review - pending
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mlepage-google @woody-apple @andy31415 @erjiaqing @yunhanw-google @bzbarsky-apple