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

[IM] Reject multiple commands in a same invoke transaction on the server side #16338

Merged

Conversation

erjiaqing
Copy link
Contributor

Problem

The protocol supports putting multiple commands in the same invoke request, but this is a feature for 1.x. We did not reject multiple commands on the server side for now.

Change overview

Add a check before handling the commands.

Testing

  • Adds TestCommandHandlerRejectMultipleCommands

@github-actions
Copy link

github-actions bot commented Mar 17, 2022

PR #16338: Size comparison from d80d593 to 9cc41ec

Increases (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section d80d593 9cc41ec change % change
cyw30739 light cyw930739m2evb_01 (read/write) 603418 603482 64 0.0
.app_xip_area 510516 510580 64 0.0
lock cyw930739m2evb_01 (read/write) 561206 561270 64 0.0
.app_xip_area 469832 469896 64 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 571010 571066 56 0.0
.app_xip_area 469988 470044 56 0.0
efr32 lighting-app BRD4161A (read only) 920940 921004 64 0.0
.text 920932 920996 64 0.0
BRD4161A+rpc (read only) 949744 949824 80 0.0
.text 949736 949816 80 0.0
window-app BRD4161A (read only) 851692 851772 80 0.0
.text 851684 851764 80 0.0
esp32 all-clusters-app c3devkit (read only) 961328 961410 82 0.0
.flash.text 961328 961410 82 0.0
m5stack (read only) 1017295 1017387 92 0.0
.flash.text 1011911 1012003 92 0.0
k32w light k32w061+release (read/write) 700096 700160 64 0.0
.text 614776 614840 64 0.0
lock k32w061+release (read/write) 700392 700456 64 0.0
.text 615060 615124 64 0.0
linux chip-tool-ipv6only arm64 (read only) 9754676 9755076 400 0.0
.text 8212228 8212628 400 0.0
thermostat-no-ble arm64 (read only) 2220364 2220780 416 0.0
.text 1862528 1862944 416 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read/write) 2352364 2352428 64 0.0
.text 1314964 1315028 64 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1154799 1154863 64 0.0
text 787376 787440 64 0.0
p6 all-clusters-app default (read/write) 2491648 2491728 80 0.0
.text 1449912 1449992 80 0.0
light-app default (read/write) 2395512 2395576 64 0.0
.text 1353776 1353840 64 0.0
lock-app default (read/write) 2359056 2359120 64 0.0
.text 1317320 1317384 64 0.0
telink lighting-app tlsr9518adk80d (read/write) 894938 895018 80 0.0
text 632758 632840 82 0.0
Full report (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section d80d593 9cc41ec change % change
cyw30739 light cyw930739m2evb_01 (read/write) 603418 603482 64 0.0
.app_xip_area 510516 510580 64 0.0
.bss 75656 75656 0 0.0
.data 596 596 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 561206 561270 64 0.0
.app_xip_area 469832 469896 64 0.0
.bss 74160 74160 0 0.0
.data 560 560 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 571010 571066 56 0.0
.app_xip_area 469988 470044 56 0.0
.bss 83464 83464 0 0.0
.data 520 520 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 920940 921004 64 0.0
(read/write) 128752 128752 0 0.0
.bss 126744 126744 0 0.0
.data 2008 2008 0 0.0
.text 920932 920996 64 0.0
BRD4161A+rpc (read only) 949744 949824 80 0.0
(read/write) 144712 144712 0 0.0
.bss 142520 142520 0 0.0
.data 2188 2188 0 0.0
.text 949736 949816 80 0.0
window-app BRD4161A (read only) 851692 851772 80 0.0
(read/write) 126704 126704 0 0.0
.bss 124840 124840 0 0.0
.data 1864 1864 0 0.0
.text 851684 851764 80 0.0
esp32 all-clusters-app c3devkit (read only) 961328 961410 82 0.0
(read/write) 1394818 1394818 0 0.0
.dram0.bss 64048 64048 0 0.0
.dram0.data 14188 14188 0 0.0
.flash.rodata 197672 197672 0 0.0
.flash.text 961328 961410 82 0.0
.iram0.text 62016 62016 0 0.0
m5stack (read only) 1017295 1017387 92 0.0
(read/write) 462484 462484 0 0.0
.dram0.bss 69576 69576 0 0.0
.dram0.data 34016 34016 0 0.0
.flash.rodata 227056 227056 0 0.0
.flash.text 1011911 1012003 92 0.0
.iram0.text 123107 123107 0 0.0
k32w light k32w061+release (read/write) 700096 700160 64 0.0
.bss 77632 77632 0 0.0
.data 1888 1888 0 0.0
.text 614776 614840 64 0.0
lock k32w061+release (read/write) 700392 700456 64 0.0
.bss 77624 77624 0 0.0
.data 1908 1908 0 0.0
.text 615060 615124 64 0.0
linux chip-tool-ipv6only arm64 (read only) 9754676 9755076 400 0.0
(read/write) 475505 475505 0 0.0
.bss 44033 44033 0 0.0
.data 1128 1128 0 0.0
.data.rel.ro 371288 371288 0 0.0
.dynamic 560 560 0 0.0
.got 55264 55264 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 492460 492460 0 0.0
.text 8212228 8212628 400 0.0
thermostat-no-ble arm64 (read only) 2220364 2220780 416 0.0
(read/write) 149809 149809 0 0.0
.bss 65713 65713 0 0.0
.data 1024 1024 0 0.0
.data.rel.ro 75728 75728 0 0.0
.dynamic 560 560 0 0.0
.got 4352 4352 0 0.0
.init 24 24 0 0.0
.init_array 360 360 0 0.0
.rodata 137772 137772 0 0.0
.text 1862528 1862944 416 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2352364 2352428 64 0.0
.bss 186644 186644 0 0.0
.data 5752 5752 0 0.0
.text 1314964 1315028 64 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1154799 1154863 64 0.0
bss 146724 146724 0 0.0
rodata 145536 145536 0 0.0
text 787376 787440 64 0.0
p6 all-clusters-app default (read/write) 2491648 2491728 80 0.0
.bss 120072 120072 0 0.0
.data 2632 2632 0 0.0
.text 1449912 1449992 80 0.0
light-app default (read/write) 2395512 2395576 64 0.0
.bss 113536 113536 0 0.0
.data 2488 2488 0 0.0
.text 1353776 1353840 64 0.0
lock-app default (read/write) 2359056 2359120 64 0.0
.bss 113280 113280 0 0.0
.data 2448 2448 0 0.0
.text 1317320 1317384 64 0.0
telink lighting-app tlsr9518adk80d (read/write) 894938 895018 80 0.0
bss 87432 87432 0 0.0
noinit 37160 37160 0 0.0
text 632758 632840 82 0.0

@cjandhyala
Copy link
Contributor

@erjiaqing can we have spec update as well so that we can update our test cases accordingly.

@bzbarsky-apple
Copy link
Contributor

@cjandhyala Per spec, multiple commands in an invoke are allowed. It's just not supported in the SDK for 1.0, just like a bunch of other things the spec allows that we don't support yet.

@andy31415 andy31415 merged commit fea74e6 into project-chip:master Mar 18, 2022
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this pull request Apr 14, 2022
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