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

Handle IM error in Read #12383

Merged

Conversation

mrjerryjohns
Copy link
Contributor

When handling the data returned on a read, we were proceeding to decode
the data even if we had just received a status code.

This resulted in confusing errors being delivered up to clients that
seemed to indicate we couldn't decode the data when in fact, we never
received any data at all and rather, just got a status code.

@github-actions
Copy link

github-actions bot commented Nov 30, 2021

PR #12383: Size comparison from fe681d9 to e60e99f

Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section fe681d9 e60e99f change % change
efr32 lighting-app BRD4161A (read only) 763096 763096 0 0.0
(read/write) 119956 119956 0 0.0
.bss 118140 118140 0 0.0
.data 1812 1812 0 0.0
.text 763088 763088 0 0.0
BRD4161A+rpc (read only) 791608 791608 0 0.0
(read/write) 138252 138252 0 0.0
.bss 136340 136340 0 0.0
.data 1912 1912 0 0.0
.text 791600 791600 0 0.0
lock-app BRD4161A (read only) 736976 736976 0 0.0
(read/write) 117660 117660 0 0.0
.bss 115892 115892 0 0.0
.data 1768 1768 0 0.0
.text 736968 736968 0 0.0
window-app BRD4161A (read only) 740080 740080 0 0.0
(read/write) 118092 118092 0 0.0
.bss 116316 116316 0 0.0
.data 1776 1776 0 0.0
.text 740072 740072 0 0.0
esp32 all-clusters-app c3devkit (read only) 837376 837376 0 0.0
(read/write) 1224450 1224450 0 0.0
.dram0.bss 58552 58552 0 0.0
.dram0.data 14028 14028 0 0.0
.flash.rodata 166616 166616 0 0.0
.flash.text 837376 837376 0 0.0
.iram0.text 61394 61394 0 0.0
m5stack (read only) 908883 908883 0 0.0
(read/write) 423684 423684 0 0.0
.dram0.bss 63952 63952 0 0.0
.dram0.data 34000 34000 0 0.0
.flash.rodata 194452 194452 0 0.0
.flash.text 903499 903499 0 0.0
.iram0.text 122943 122943 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 724508 724508 0 0.0
.bss 78772 78772 0 0.0
.data 1840 1840 0 0.0
.text 638096 638096 0 0.0
lock-app k32w061+debug (read/write) 613492 613492 0 0.0
.bss 69212 69212 0 0.0
.data 1804 1804 0 0.0
.text 536676 536676 0 0.0
shell k32w061+debug (read/write) 679288 679288 0 0.0
.bss 80796 80796 0 0.0
.data 1776 1776 0 0.0
.text 590916 590916 0 0.0
linux all-clusters-app debug (read only) 1780409 1780409 0 0.0
(read/write) 125288 125288 0 0.0
.bss 53104 53104 0 0.0
.data 1104 1104 0 0.0
.data.rel.ro 65792 65792 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 139957 139957 0 0.0
.text 1500738 1500738 0 0.0
bridge-app debug+rpc (read only) 1352373 1352373 0 0.0
(read/write) 71312 71312 0 0.0
.bss 35120 35120 0 0.0
.data 1680 1680 0 0.0
.data.rel.ro 29488 29488 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 113980 113980 0 0.0
.text 1138181 1138181 0 0.0
chip-tool debug (read only) 6247237 6247237 0 0.0
(read/write) 197648 197648 0 0.0
.bss 33448 33448 0 0.0
.data 1008 1008 0 0.0
.data.rel.ro 157608 157608 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 294984 294984 0 0.0
.text 5549909 5549909 0 0.0
lighting-app debug+rpc (read only) 1632441 1632441 0 0.0
(read/write) 104400 104400 0 0.0
.bss 40784 40784 0 0.0
.data 1232 1232 0 0.0
.data.rel.ro 57072 57072 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 132433 132433 0 0.0
.text 1362098 1362098 0 0.0
ota-provider-app debug (read only) 1312817 1312817 0 0.0
(read/write) 69768 69768 0 0.0
.bss 37696 37696 0 0.0
.data 912 912 0 0.0
.data.rel.ro 26024 26024 0 0.0
.dynamic 592 592 0 0.0
.got 4048 4048 0 0.0
.init 27 27 0 0.0
.init_array 448 448 0 0.0
.rodata 115184 115184 0 0.0
.text 1097186 1097186 0 0.0
ota-requestor-app debug (read only) 1412801 1412801 0 0.0
(read/write) 73664 73664 0 0.0
.bss 39808 39808 0 0.0
.data 976 976 0 0.0
.data.rel.ro 27736 27736 0 0.0
.dynamic 592 592 0 0.0
.got 4032 4032 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 127008 127008 0 0.0
.text 1181906 1181906 0 0.0
shell debug (read only) 805881 805881 0 0.0
(read/write) 59848 59848 0 0.0
.bss 16808 16808 0 0.0
.data 240 240 0 0.0
.data.rel.ro 38320 38320 0 0.0
.dynamic 592 592 0 0.0
.got 3504 3504 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 79954 79954 0 0.0
.text 620610 620610 0 0.0
tv-app debug (read only) 1927025 1927025 0 0.0
(read/write) 314032 314032 0 0.0
.bss 245176 245176 0 0.0
.data 1504 1504 0 0.0
.data.rel.ro 61696 61696 0 0.0
.dynamic 592 592 0 0.0
.got 4424 4424 0 0.0
.init 27 27 0 0.0
.init_array 624 624 0 0.0
.rodata 161576 161576 0 0.0
.text 1616914 1616914 0 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2296640 2296640 0 0.0
.bss 181636 181636 0 0.0
.data 5168 5168 0 0.0
.heap 849640 849640 0 0.0
.text 1259240 1259240 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2281312 2281312 0 0.0
.bss 172636 172636 0 0.0
.data 5480 5480 0 0.0
.heap 858328 858328 0 0.0
.text 1243912 1243912 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2254344 2254344 0 0.0
.bss 171452 171452 0 0.0
.data 5464 5464 0 0.0
.heap 859528 859528 0 0.0
.text 1216944 1216944 0 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) 2046096 2046096 0 0.0
.bss 156592 156592 0 0.0
.data 4864 4864 0 0.0
.heap 874992 874992 0 0.0
.text 1008696 1008696 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 876791 876791 0 0.0
bss 113136 113136 0 0.0
rodata 97368 97368 0 0.0
text 590740 590740 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 839239 839239 0 0.0
bss 109488 109488 0 0.0
rodata 88632 88632 0 0.0
text 564908 564908 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 801810 801810 0 0.0
bss 114512 114512 0 0.0
rodata 92628 92628 0 0.0
text 520196 520196 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 847603 847603 0 0.0
bss 110176 110176 0 0.0
rodata 93108 93108 0 0.0
text 568988 568988 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 772874 772874 0 0.0
bss 111584 111584 0 0.0
rodata 88396 88396 0 0.0
text 498536 498536 0 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) 853647 853647 0 0.0
bss 110312 110312 0 0.0
rodata 94840 94840 0 0.0
text 573052 573052 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 846711 846711 0 0.0
bss 110188 110188 0 0.0
rodata 92976 92976 0 0.0
text 568112 568112 0 0.0
shell nrf52840dk_nrf52840 (read/write) 779163 779163 0 0.0
bss 109616 109616 0 0.0
rodata 73216 73216 0 0.0
text 521832 521832 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 694210 694210 0 0.0
bss 110600 110600 0 0.0
rodata 67860 67860 0 0.0
text 442444 442444 0 0.0
p6 all-clusters-app default (read/write) 2318376 2318376 0 0.0
.bss 109712 109712 0 0.0
.data 2456 2456 0 0.0
.heap 921176 921176 0 0.0
.text 1276640 1276640 0 0.0
lock-app default (read/write) 2229528 2229528 0 0.0
.bss 96408 96408 0 0.0
.data 2280 2280 0 0.0
.heap 934656 934656 0 0.0
.text 1187792 1187792 0 0.0
qpg lighting-app qpg6100+debug (read only) 496860 496860 0 0.0
(read/write) 114140 114140 0 0.0
.bss 79656 79656 0 0.0
.data 940 940 0 0.0
.text 491540 491540 0 0.0
lock-app qpg6100+debug (read only) 469448 469448 0 0.0
(read/write) 114140 114140 0 0.0
.bss 78568 78568 0 0.0
.data 892 892 0 0.0
.text 464128 464128 0 0.0
persistent-storage-app qpg6100+debug (read only) 108020 108020 0 0.0
(read/write) 114140 114140 0 0.0
.bss 36688 36688 0 0.0
.data 292 292 0 0.0
.text 102700 102700 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 778614 778614 0 0.0
bss 79716 79716 0 0.0
noinit 37160 37160 0 0.0
text 541394 541394 0 0.0

@andy31415
Copy link
Contributor

/rebase

When handling the data returned on a read, we were proceeding to decode
the data even if we had just received a status code.

This resulted in confusing errors being delivered up to clients that
seemed to indicate we couldn't decode the data when in fact, we never
received any data at all and rather, just got a status code.
@woody-apple woody-apple force-pushed the python/handle-im-error-read branch from e60e99f to 646f9e3 Compare December 1, 2021 19:13
@github-actions
Copy link

github-actions bot commented Dec 1, 2021

PR #12383: Size comparison from 7b7374a to 646f9e3

Full report (39 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 7b7374a 646f9e3 change % change
efr32 lighting-app BRD4161A (read only) 727448 727448 0 0.0
(read/write) 119420 119420 0 0.0
.bss 117604 117604 0 0.0
.data 1812 1812 0 0.0
.text 727440 727440 0 0.0
BRD4161A+rpc (read only) 756088 756088 0 0.0
(read/write) 137724 137724 0 0.0
.bss 135804 135804 0 0.0
.data 1920 1920 0 0.0
.text 756080 756080 0 0.0
lock-app BRD4161A (read only) 701308 701308 0 0.0
(read/write) 117124 117124 0 0.0
.bss 115356 115356 0 0.0
.data 1768 1768 0 0.0
.text 701300 701300 0 0.0
window-app BRD4161A (read only) 704420 704420 0 0.0
(read/write) 117548 117548 0 0.0
.bss 115772 115772 0 0.0
.data 1776 1776 0 0.0
.text 704412 704412 0 0.0
esp32 all-clusters-app c3devkit (read only) 837840 837840 0 0.0
(read/write) 1224178 1224178 0 0.0
.dram0.bss 58560 58560 0 0.0
.dram0.data 14028 14028 0 0.0
.flash.rodata 166344 166344 0 0.0
.flash.text 837840 837840 0 0.0
.iram0.text 61394 61394 0 0.0
m5stack (read only) 909347 909347 0 0.0
(read/write) 423440 423440 0 0.0
.dram0.bss 63960 63960 0 0.0
.dram0.data 34000 34000 0 0.0
.flash.rodata 194200 194200 0 0.0
.flash.text 903963 903963 0 0.0
.iram0.text 122943 122943 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 725032 725032 0 0.0
.bss 78796 78796 0 0.0
.data 1844 1844 0 0.0
.text 638592 638592 0 0.0
lock-app k32w061+debug (read/write) 614032 614032 0 0.0
.bss 69236 69236 0 0.0
.data 1808 1808 0 0.0
.text 537188 537188 0 0.0
shell k32w061+debug (read/write) 679828 679828 0 0.0
.bss 80836 80836 0 0.0
.data 1780 1780 0 0.0
.text 591412 591412 0 0.0
linux all-clusters-app debug (read only) 1788777 1788777 0 0.0
(read/write) 125480 125480 0 0.0
.bss 53104 53104 0 0.0
.data 1104 1104 0 0.0
.data.rel.ro 65968 65968 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 144597 144597 0 0.0
.text 1504082 1504082 0 0.0
bridge-app debug+rpc (read only) 1358845 1358845 0 0.0
(read/write) 71568 71568 0 0.0
.bss 35152 35152 0 0.0
.data 1680 1680 0 0.0
.data.rel.ro 29728 29728 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 117636 117636 0 0.0
.text 1140517 1140517 0 0.0
chip-tool debug (read only) 6428085 6428085 0 0.0
(read/write) 198192 198192 0 0.0
.bss 33448 33448 0 0.0
.data 1008 1008 0 0.0
.data.rel.ro 158152 158152 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 300152 300152 0 0.0
.text 5724245 5724245 0 0.0
lighting-app debug+rpc (read only) 1642649 1642649 0 0.0
(read/write) 104624 104624 0 0.0
.bss 40816 40816 0 0.0
.data 1264 1264 0 0.0
.data.rel.ro 57232 57232 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 137297 137297 0 0.0
.text 1366914 1366914 0 0.0
ota-provider-app debug (read only) 1319433 1319433 0 0.0
(read/write) 70024 70024 0 0.0
.bss 37728 37728 0 0.0
.data 912 912 0 0.0
.data.rel.ro 26264 26264 0 0.0
.dynamic 592 592 0 0.0
.got 4048 4048 0 0.0
.init 27 27 0 0.0
.init_array 448 448 0 0.0
.rodata 118984 118984 0 0.0
.text 1099522 1099522 0 0.0
ota-requestor-app debug (read only) 1419537 1419537 0 0.0
(read/write) 73920 73920 0 0.0
.bss 39840 39840 0 0.0
.data 976 976 0 0.0
.data.rel.ro 27976 27976 0 0.0
.dynamic 592 592 0 0.0
.got 4032 4032 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 130928 130928 0 0.0
.text 1184242 1184242 0 0.0
shell debug (read only) 814233 814233 0 0.0
(read/write) 60232 60232 0 0.0
.bss 16872 16872 0 0.0
.data 240 240 0 0.0
.data.rel.ro 38656 38656 0 0.0
.dynamic 592 592 0 0.0
.got 3504 3504 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 83506 83506 0 0.0
.text 624642 624642 0 0.0
tv-app debug (read only) 1935153 1935153 0 0.0
(read/write) 314288 314288 0 0.0
.bss 245208 245208 0 0.0
.data 1504 1504 0 0.0
.data.rel.ro 61920 61920 0 0.0
.dynamic 592 592 0 0.0
.got 4424 4424 0 0.0
.init 27 27 0 0.0
.init_array 624 624 0 0.0
.rodata 166856 166856 0 0.0
.text 1619282 1619282 0 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2296920 2296920 0 0.0
.bss 181628 181628 0 0.0
.data 5168 5168 0 0.0
.heap 849648 849648 0 0.0
.text 1259520 1259520 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2281672 2281672 0 0.0
.bss 172656 172656 0 0.0
.data 5480 5480 0 0.0
.heap 858312 858312 0 0.0
.text 1244272 1244272 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2254704 2254704 0 0.0
.bss 171472 171472 0 0.0
.data 5480 5480 0 0.0
.heap 859496 859496 0 0.0
.text 1217304 1217304 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) 2045944 2045944 0 0.0
.bss 156624 156624 0 0.0
.data 4864 4864 0 0.0
.heap 874960 874960 0 0.0
.text 1008544 1008544 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 877159 877159 0 0.0
bss 113156 113156 0 0.0
rodata 97464 97464 0 0.0
text 590976 590976 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 839591 839591 0 0.0
bss 109504 109504 0 0.0
rodata 88824 88824 0 0.0
text 565076 565076 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 802146 802146 0 0.0
bss 114528 114528 0 0.0
rodata 92724 92724 0 0.0
text 520432 520432 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 847971 847971 0 0.0
bss 110192 110192 0 0.0
rodata 93204 93204 0 0.0
text 569224 569224 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 773242 773242 0 0.0
bss 111604 111604 0 0.0
rodata 88492 88492 0 0.0
text 498772 498772 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) 853983 853983 0 0.0
bss 110328 110328 0 0.0
rodata 94936 94936 0 0.0
text 573292 573292 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 847063 847063 0 0.0
bss 110208 110208 0 0.0
rodata 93072 93072 0 0.0
text 568352 568352 0 0.0
shell nrf52840dk_nrf52840 (read/write) 779263 779263 0 0.0
bss 109616 109616 0 0.0
rodata 73236 73236 0 0.0
text 521912 521912 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 694310 694310 0 0.0
bss 110600 110600 0 0.0
rodata 67880 67880 0 0.0
text 442520 442520 0 0.0
p6 all-clusters-app default (read/write) 2318616 2318616 0 0.0
.bss 109712 109712 0 0.0
.data 2464 2464 0 0.0
.heap 921168 921168 0 0.0
.text 1276880 1276880 0 0.0
light-app default (read/write) 2254344 2254344 0 0.0
.bss 97776 97776 0 0.0
.data 2328 2328 0 0.0
.heap 933240 933240 0 0.0
.text 1212608 1212608 0 0.0
lock-app default (read/write) 2229688 2229688 0 0.0
.bss 96432 96432 0 0.0
.data 2288 2288 0 0.0
.heap 934624 934624 0 0.0
.text 1187952 1187952 0 0.0
qpg lighting-app qpg6100+debug (read only) 497372 497372 0 0.0
(read/write) 114144 114144 0 0.0
.bss 79688 79688 0 0.0
.data 944 944 0 0.0
.text 492052 492052 0 0.0
lock-app qpg6100+debug (read only) 469960 469960 0 0.0
(read/write) 114144 114144 0 0.0
.bss 78600 78600 0 0.0
.data 896 896 0 0.0
.text 464640 464640 0 0.0
persistent-storage-app qpg6100+debug (read only) 108208 108208 0 0.0
(read/write) 114140 114140 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) 779054 779054 0 0.0
bss 79736 79736 0 0.0
noinit 37160 37160 0 0.0
text 541714 541714 0 0.0

@boring-cyborg boring-cyborg bot added the app label Dec 2, 2021
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.

The test-cluster-server bit looks ok, but as a followup we should really make the write work too, and then just return whatever value we have stored.

@andy31415 andy31415 merged commit b6adaf7 into project-chip:master Dec 2, 2021
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.

5 participants