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

Add strict IM encoding ordering check #12549

Merged
merged 1 commit into from
Dec 3, 2021
Merged

Add strict IM encoding ordering check #12549

merged 1 commit into from
Dec 3, 2021

Conversation

yunhanw-google
Copy link
Contributor

@yunhanw-google yunhanw-google commented Dec 3, 2021 •
edited
Loading

Problem

in "10.5.1. Tag Rules":
Unless otherwise noted, all context tags SHALL be emitted in the order as defined in the appropriate specification.

We need apply strict tag ordering for IM encoding per spec.

Change overview

--Add StructParser::CheckSchemaOrdering function and Apply strict tag ordering for IM encoding per spec
--Fix the ordering issue across all IM codes.
--This PR also cover the tag ordering change for this spec ticket, https://github.com/CHIP-Specifications/connectedhomeip-spec/issues/4730

Testing

The existing test covers.

@todo
Copy link

todo bot commented Dec 3, 2021

Add attribute version support

connectedhomeip/src/app/WriteClient.cpp

Lines 141 to 146 in 88ad6bd

// TODO: Add attribute version support
attributeDataIB.DataVersion(0);
ReturnErrorOnFailure(attributeDataIB.GetError());
ReturnErrorOnFailure(attributeDataIB.CreatePath().Encode(attributePathParams));
return CHIP_NO_ERROR;
}

This comment was generated by todo based on a TODO comment in 88ad6bd in #12549. cc @yunhanw-google.

@github-actions
Copy link

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

PR #12549: Size comparison from 70af2c4 to 88ad6bd

Increases (32 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 70af2c4 88ad6bde change % change
efr32 lighting-app BRD4161A (read only) 751848 751928 80 0.0
.text 751840 751920 80 0.0
BRD4161A+rpc (read only) 780512 780600 88 0.0
.text 780504 780592 88 0.0
lock-app BRD4161A (read only) 725704 725784 80 0.0
.text 725696 725776 80 0.0
window-app BRD4161A (read only) 729016 729096 80 0.0
.text 729008 729088 80 0.0
esp32 all-clusters-app c3devkit (read only) 847956 848056 100 0.0
.flash.text 847956 848056 100 0.0
m5stack (read only) 918355 918463 108 0.0
.flash.text 912971 913079 108 0.0
k32w lighting-app k32w061+se05x+release (read/write) 728748 728824 76 0.0
.text 641776 641852 76 0.0
lock-app k32w061+debug (read/write) 617700 617792 92 0.0
.text 540324 540416 92 0.0
shell k32w061+debug (read/write) 683608 683684 76 0.0
.text 594612 594688 76 0.0
linux all-clusters-app debug (read only) 1848689 1849569 880 0.0
.text 1557074 1557986 912 0.1
bridge-app debug+rpc (read only) 1426837 1427749 912 0.1
.text 1201093 1202005 912 0.1
chip-tool debug (read only) 6684229 6685221 992 0.0
.text 5964965 5965989 1024 0.0
lighting-app debug+rpc (read only) 1711985 1712865 880 0.1
.text 1427794 1428706 912 0.1
ota-provider-app debug (read only) 1386889 1387769 880 0.1
.text 1160594 1161506 912 0.1
ota-requestor-app debug (read only) 1493337 1494441 1104 0.1
.text 1250546 1251682 1136 0.1
tv-app debug (read only) 2022841 2023945 1104 0.1
.text 1694402 1695538 1136 0.1
mbed all-clusters-app CY8CPROTO_062_4343W+release (read/write) 2309544 2309584 40 0.0
.text 1272120 1272160 40 0.0
lighting-app CY8CPROTO_062_4343W+release (read/write) 2295872 2295976 104 0.0
.text 1258472 1258576 104 0.0
lock-app CY8CPROTO_062_4343W+release (read/write) 2268904 2269008 104 0.0
.text 1231504 1231608 104 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 891767 891843 76 0.0
text 602860 602956 96 0.0
nrf52840dk_nrf52840+rpc (read/write) 855143 855219 76 0.0
text 577856 577952 96 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 817678 817754 76 0.0
text 533196 533296 100 0.0
lock-app nrf52840dk_nrf52840 (read/write) 862591 862667 76 0.0
text 581080 581176 96 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 788746 788822 76 0.0
text 511508 511608 100 0.0
pump-app nrf52840dk_nrf52840 (read/write) 868607 868683 76 0.0
text 585144 585244 100 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 861671 861763 92 0.0
text 580204 580304 100 0.0
p6 all-clusters-app default (read/write) 2339800 2339888 88 0.0
.text 1298064 1298152 88 0.0
light-app default (read/write) 2279168 2279248 80 0.0
.text 1237432 1237512 80 0.0
lock-app default (read/write) 2254504 2254600 96 0.0
.text 1212768 1212864 96 0.0
qpg lighting-app qpg6100+debug (read only) 510540 510624 84 0.0
.text 505220 505304 84 0.0
lock-app qpg6100+debug (read only) 483308 483392 84 0.0
.text 477988 478072 84 0.0
telink lighting-app tlsr9518adk80d (read/write) 794862 794946 84 0.0
text 554638 554738 100 0.0
Decreases (15 builds for esp32, linux, nrfconnect)
platform target config section 70af2c4 88ad6bde change % change
esp32 all-clusters-app c3devkit (read/write) 1224026 1224002 -24 -0.0
.flash.rodata 167944 167920 -24 -0.0
m5stack (read/write) 422996 422972 -24 -0.0
.flash.rodata 195740 195716 -24 -0.0
linux all-clusters-app debug .rodata 148725 148693 -32 -0.0
chip-tool debug .rodata 308504 308472 -32 -0.0
lighting-app debug+rpc .rodata 141521 141489 -32 -0.0
ota-provider-app debug .rodata 121608 121576 -32 -0.0
ota-requestor-app debug .rodata 134096 134064 -32 -0.0
tv-app debug .rodata 173960 173928 -32 -0.0
nrfconnect lighting-app nrf52840dk_nrf52840 rodata 99608 99588 -20 -0.0
nrf52840dk_nrf52840+rpc rodata 90968 90948 -20 -0.0
nrf5340dk_nrf5340_cpuapp rodata 94864 94844 -20 -0.0
lock-app nrf52840dk_nrf52840 rodata 95344 95324 -20 -0.0
nrf5340dk_nrf5340_cpuapp rodata 90636 90616 -20 -0.0
pump-app nrf52840dk_nrf52840 rodata 97080 97060 -20 -0.0
pump-controller-app nrf52840dk_nrf52840 rodata 95216 95196 -20 -0.0
Full report (39 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 70af2c4 88ad6bde change % change
efr32 lighting-app BRD4161A (read only) 751848 751928 80 0.0
(read/write) 120032 120032 0 0.0
.bss 118200 118200 0 0.0
.data 1828 1828 0 0.0
.text 751840 751920 80 0.0
BRD4161A+rpc (read only) 780512 780600 88 0.0
(read/write) 138336 138336 0 0.0
.bss 136400 136400 0 0.0
.data 1936 1936 0 0.0
.text 780504 780592 88 0.0
lock-app BRD4161A (read only) 725704 725784 80 0.0
(read/write) 117736 117736 0 0.0
.bss 115952 115952 0 0.0
.data 1784 1784 0 0.0
.text 725696 725776 80 0.0
window-app BRD4161A (read only) 729016 729096 80 0.0
(read/write) 118168 118168 0 0.0
.bss 116376 116376 0 0.0
.data 1792 1792 0 0.0
.text 729008 729088 80 0.0
esp32 all-clusters-app c3devkit (read only) 847956 848056 100 0.0
(read/write) 1224026 1224002 -24 -0.0
.dram0.bss 56576 56576 0 0.0
.dram0.data 14052 14052 0 0.0
.flash.rodata 167944 167920 -24 -0.0
.flash.text 847956 848056 100 0.0
.iram0.text 61394 61394 0 0.0
m5stack (read only) 918355 918463 108 0.0
(read/write) 422996 422972 -24 -0.0
.dram0.bss 61960 61960 0 0.0
.dram0.data 34016 34016 0 0.0
.flash.rodata 195740 195716 -24 -0.0
.flash.text 912971 913079 108 0.0
.iram0.text 122943 122943 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 728748 728824 76 0.0
.bss 79312 79312 0 0.0
.data 1860 1860 0 0.0
.text 641776 641852 76 0.0
lock-app k32w061+debug (read/write) 617700 617792 92 0.0
.bss 69752 69752 0 0.0
.data 1824 1824 0 0.0
.text 540324 540416 92 0.0
shell k32w061+debug (read/write) 683608 683684 76 0.0
.bss 81400 81400 0 0.0
.data 1796 1796 0 0.0
.text 594612 594688 76 0.0
linux all-clusters-app debug (read only) 1848689 1849569 880 0.0
(read/write) 124520 124520 0 0.0
.bss 50896 50896 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 688 688 0 0.0
.rodata 148725 148693 -32 -0.0
.text 1557074 1557986 912 0.1
bridge-app debug+rpc (read only) 1426837 1427749 912 0.1
(read/write) 74616 74616 0 0.0
.bss 36240 36240 0 0.0
.data 1728 1728 0 0.0
.data.rel.ro 31560 31560 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 120844 120844 0 0.0
.text 1201093 1202005 912 0.1
chip-tool debug (read only) 6684229 6685221 992 0.0
(read/write) 202376 202376 0 0.0
.bss 34504 34504 0 0.0
.data 1024 1024 0 0.0
.data.rel.ro 161208 161208 0 0.0
.dynamic 592 592 0 0.0
.got 4472 4472 0 0.0
.init 27 27 0 0.0
.init_array 568 568 0 0.0
.rodata 308504 308472 -32 -0.0
.text 5964965 5965989 1024 0.0
lighting-app debug+rpc (read only) 1711985 1712865 880 0.1
(read/write) 107648 107648 0 0.0
.bss 41936 41936 0 0.0
.data 1280 1280 0 0.0
.data.rel.ro 59056 59056 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 141521 141489 -32 -0.0
.text 1427794 1428706 912 0.1
ota-provider-app debug (read only) 1386889 1387769 880 0.1
(read/write) 72816 72816 0 0.0
.bss 38816 38816 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 520 520 0 0.0
.rodata 121608 121576 -32 -0.0
.text 1160594 1161506 912 0.1
ota-requestor-app debug (read only) 1493337 1494441 1104 0.1
(read/write) 76784 76784 0 0.0
.bss 40960 40960 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 552 552 0 0.0
.rodata 134096 134064 -32 -0.0
.text 1250546 1251682 1136 0.1
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) 2022841 2023945 1104 0.1
(read/write) 320000 320000 0 0.0
.bss 247224 247224 0 0.0
.data 2768 2768 0 0.0
.data.rel.ro 64200 64200 0 0.0
.dynamic 592 592 0 0.0
.got 4432 4432 0 0.0
.init 27 27 0 0.0
.init_array 736 736 0 0.0
.rodata 173960 173928 -32 -0.0
.text 1694402 1695538 1136 0.1
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2309544 2309584 40 0.0
.bss 179636 179636 0 0.0
.data 5184 5184 0 0.0
.heap 851624 851624 0 0.0
.text 1272120 1272160 40 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2295872 2295976 104 0.0
.bss 173272 173272 0 0.0
.data 5496 5496 0 0.0
.heap 857680 857680 0 0.0
.text 1258472 1258576 104 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2268904 2269008 104 0.0
.bss 172088 172088 0 0.0
.data 5496 5496 0 0.0
.heap 858864 858864 0 0.0
.text 1231504 1231608 104 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) 891767 891843 76 0.0
bss 113756 113756 0 0.0
rodata 99608 99588 -20 -0.0
text 602860 602956 96 0.0
nrf52840dk_nrf52840+rpc (read/write) 855143 855219 76 0.0
bss 110104 110104 0 0.0
rodata 90968 90948 -20 -0.0
text 577856 577952 96 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 817678 817754 76 0.0
bss 115128 115128 0 0.0
rodata 94864 94844 -20 -0.0
text 533196 533296 100 0.0
lock-app nrf52840dk_nrf52840 (read/write) 862591 862667 76 0.0
bss 110792 110792 0 0.0
rodata 95344 95324 -20 -0.0
text 581080 581176 96 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 788746 788822 76 0.0
bss 112204 112204 0 0.0
rodata 90636 90616 -20 -0.0
text 511508 511608 100 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) 868607 868683 76 0.0
bss 110928 110928 0 0.0
rodata 97080 97060 -20 -0.0
text 585144 585244 100 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 861671 861763 92 0.0
bss 110808 110808 0 0.0
rodata 95216 95196 -20 -0.0
text 580204 580304 100 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) 2339800 2339888 88 0.0
.bss 107820 107820 0 0.0
.data 2456 2456 0 0.0
.heap 923064 923064 0 0.0
.text 1298064 1298152 88 0.0
light-app default (read/write) 2279168 2279248 80 0.0
.bss 98504 98504 0 0.0
.data 2336 2336 0 0.0
.heap 932504 932504 0 0.0
.text 1237432 1237512 80 0.0
lock-app default (read/write) 2254504 2254600 96 0.0
.bss 97160 97160 0 0.0
.data 2296 2296 0 0.0
.heap 933888 933888 0 0.0
.text 1212768 1212864 96 0.0
qpg lighting-app qpg6100+debug (read only) 510540 510624 84 0.0
(read/write) 122332 122332 0 0.0
.bss 80272 80272 0 0.0
.data 964 964 0 0.0
.text 505220 505304 84 0.0
lock-app qpg6100+debug (read only) 483308 483392 84 0.0
(read/write) 122332 122332 0 0.0
.bss 79184 79184 0 0.0
.data 916 916 0 0.0
.text 477988 478072 84 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) 794862 794946 84 0.0
bss 80332 80332 0 0.0
noinit 37160 37160 0 0.0
text 554638 554738 100 0.0

@andy31415 andy31415 merged commit 51db97b into project-chip:master Dec 3, 2021
billwatersiii pushed a commit to billwatersiii/connectedhomeip that referenced this pull request Dec 3, 2021
@yunhanw-google @billwatersiii
Add strict IM encoding ordering check (project-chip#12549)
4d148f4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yufengwangca yufengwangca yufengwangca approved these changes

@andy31415 andy31415 Awaiting requested review from andy31415

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

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

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

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

@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

@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

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@lzgrablic02 lzgrablic02 Awaiting requested review from lzgrablic02

@mlepage-google mlepage-google Awaiting requested review from mlepage-google

@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

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@tecimovic tecimovic Awaiting requested review from tecimovic

@vijs vijs Awaiting requested review from vijs

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

@wbschiller wbschiller Awaiting requested review from wbschiller

@LuDuda LuDuda Awaiting requested review from LuDuda

@woody-apple woody-apple Awaiting requested review from woody-apple

Assignees
No one assigned
Labels
app controller review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yunhanw-google @LuDuda @yufengwangca @woody-apple @andy31415