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

Fix sending of timed invoke via MTRBaseDevice invokeCommandWithEndpointId #21804

Merged

Conversation

bzbarsky-apple
Copy link
Contributor

  • Fix CommandSender to error out early if it detects that its "am I a timed
    invoke" state does not match whether a timed invoke timeout was provided.
    This should prevent invalid message sequences from being sent by accident.
  • Move the one actual test from MTRClustersTests to MTRDeviceTests.
  • Enable running MTRDeviceTests in CI.
  • Add a test to MTRDeviceTests that does a timed invoke.
  • Fix incorrect initialization of CommandSender in invokeCommandWithEndpointId

The tests being enabled fail without that last item.

Problem

API not sending timed invoke correctly.

Change overview

Fix API, add tests.

Testing

Unit tests added.

…ntId.

* Fix CommandSender to error out early if it detects that its "am I a timed
  invoke" state does not match whether a timed invoke timeout was provided.
  This should prevent invalid message sequences from being sent by accident.
* Move the one actual test from MTRClustersTests to MTRDeviceTests.
* Enable running MTRDeviceTests in CI.
* Add a test to MTRDeviceTests that does a timed invoke.
* Fix incorrect initialization of CommandSender in invokeCommandWithEndpointId

The tests being enabled fail without that last item.
@github-actions
Copy link

github-actions bot commented Aug 10, 2022

PR #21804: Size comparison from 57d8d89 to 8334c7f

Increases (36 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, linux, nrfconnect, p6, telink)
platform target config section 57d8d89 8334c7f change % change
bl602 lighting-app bl602 (read/write) 1380570 1380706 136 0.0
.text 1048728 1048756 28 0.0
bl602+rpc (read/write) 1425818 1425962 144 0.0
.text 1080228 1080256 28 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 671343 671471 128 0.0
.rodata 88471 88583 112 0.1
.text 582556 582572 16 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 636743 636879 136 0.0
.rodata 77727 77839 112 0.1
.text 558692 558716 24 0.0
lock-ftd LP_CC2652R7 (read only) 673051 673187 136 0.0
.rodata 76411 76523 112 0.1
.text 596160 596184 24 0.0
lock-mtd LP_CC2652R7 (read only) 655667 655803 136 0.0
.rodata 101363 101475 112 0.1
.text 553824 553848 24 0.0
pump-app LP_CC2652R7 (read only) 683307 683435 128 0.0
.rodata 89315 89419 104 0.1
.text 593508 593532 24 0.0
pump-controller-app LP_CC2652R7 (read only) 667759 667887 128 0.0
.rodata 84863 84967 104 0.1
.text 582416 582440 24 0.0
shell LP_CC2652R7 (read only) 664018 664154 136 0.0
.rodata 85418 85530 112 0.1
.text 578284 578308 24 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 585118 585126 8 0.0
.app_xip_area 461904 461912 8 0.0
lock cyw930739m2evb_01 (read/write) 590918 590926 8 0.0
.app_xip_area 462920 462928 8 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 597466 597602 136 0.0
.app_xip_area 474972 475108 136 0.0
efr32 lighting-app BRD4161A (read/write) 1097132 1097276 144 0.0
.text 962180 962324 144 0.0
BRD4161A+rpc (read/write) 1151412 1151556 144 0.0
.text 999564 999708 144 0.0
BRD4161A+rs911x (read/write) 986644 986788 144 0.0
.text 822368 822512 144 0.0
lock-app BRD4161A+wf200 (read/write) 1136980 1137124 144 0.0
.text 990056 990200 144 0.0
window-app BRD4161A (read/write) 1088584 1088728 144 0.0
.text 952176 952320 144 0.0
esp32 all-clusters-app c3devkit (read only) 1026616 1026644 28 0.0
(read/write) 1487482 1487594 112 0.0
.flash.rodata 217080 217192 112 0.1
.flash.text 1026616 1026644 28 0.0
m5stack (read only) 1079907 1079935 28 0.0
(read/write) 489440 489552 112 0.0
.flash.rodata 247436 247548 112 0.0
.flash.text 1074523 1074551 28 0.0
linux all-clusters-app debug (read only) 3027977 3028217 240 0.0
.rodata 273547 273643 96 0.0
.text 2575474 2575618 144 0.0
all-clusters-minimal-app debug (read only) 2867769 2868041 272 0.0
.rodata 273675 273803 128 0.0
.text 2417842 2417986 144 0.0
chip-tool debug (read only) 10605713 10605953 240 0.0
.rodata 543989 544085 96 0.0
.text 8560564 8560708 144 0.0
chip-tool-ipv6only arm64 (read only) 10014540 10014796 256 0.0
.rodata 474444 474556 112 0.0
.text 7907908 7908052 144 0.0
lighting-app debug+rpc (read only) 2590849 2590945 96 0.0
.rodata 219280 219376 96 0.0
lock-app debug (read only) 2574793 2574921 128 0.0
.rodata 236368 236496 128 0.1
ota-requestor-app debug (read only) 2517425 2517681 256 0.0
.rodata 215040 215168 128 0.1
.text 2129938 2130066 128 0.0
shell debug (read only) 2596345 2596601 256 0.0
.rodata 233650 233778 128 0.1
.text 2204466 2204594 128 0.0
thermostat-no-ble arm64 (read only) 2351772 2352012 240 0.0
.rodata 139716 139828 112 0.1
.text 1974528 1974656 128 0.0
tv-app debug (read only) 3170041 3170297 256 0.0
.rodata 258248 258376 128 0.0
.text 2722002 2722130 128 0.0
tv-casting-app debug (read only) 5410729 5410985 256 0.0
.rodata 341521 341649 128 0.0
.text 4800882 4801010 128 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1178359 1178503 144 0.0
rodata 142776 142888 112 0.1
text 813520 813544 24 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1158383 1158523 140 0.0
rodata 134472 134580 108 0.1
text 802508 802532 24 0.0
p6 all-clusters-app default (read/write) 1694476 1694620 144 0.0
.text 1534256 1534400 144 0.0
all-clusters-minimal-app default (read/write) 1638516 1638660 144 0.0
.text 1479016 1479160 144 0.0
telink light-switch-app tlsr9518adk80d (read/write) 806472 806616 144 0.0
text 570178 570206 28 0.0
lighting-app tlsr9518adk80d (read/write) 828440 828576 136 0.0
text 588294 588320 26 0.0
Decreases (6 builds for cc13x2_26x2)
platform target config section 57d8d89 8334c7f change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read/write) 180048 179920 -128 -0.1
lock-ftd LP_CC2652R7 (read/write) 168532 168396 -136 -0.1
lock-mtd LP_CC2652R7 (read/write) 181604 181468 -136 -0.1
pump-app LP_CC2652R7 (read/write) 159084 158956 -128 -0.1
pump-controller-app LP_CC2652R7 (read/write) 174752 174624 -128 -0.1
shell LP_CC2652R7 (read/write) 182892 182756 -136 -0.1
Full report (43 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 57d8d89 8334c7f change % change
bl602 lighting-app bl602 (read/write) 1380570 1380706 136 0.0
.bss 119762 119762 0 0.0
.data 4480 4480 0 0.0
.text 1048728 1048756 28 0.0
bl602+rpc (read/write) 1425818 1425962 144 0.0
.bss 127202 127202 0 0.0
.data 4600 4600 0 0.0
.text 1080228 1080256 28 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 671343 671471 128 0.0
(read/write) 180048 179920 -128 -0.1
.bss 74284 74284 0 0.0
.data 3372 3372 0 0.0
.rodata 88471 88583 112 0.1
.text 582556 582572 16 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 636743 636879 136 0.0
(read/write) 157860 157860 0 0.0
.bss 73572 73572 0 0.0
.data 3372 3372 0 0.0
.rodata 77727 77839 112 0.1
.text 558692 558716 24 0.0
lock-ftd LP_CC2652R7 (read only) 673051 673187 136 0.0
(read/write) 168532 168396 -136 -0.1
.bss 71364 71364 0 0.0
.data 3296 3296 0 0.0
.rodata 76411 76523 112 0.1
.text 596160 596184 24 0.0
lock-mtd LP_CC2652R7 (read only) 655667 655803 136 0.0
(read/write) 181604 181468 -136 -0.1
.bss 67052 67052 0 0.0
.data 3296 3296 0 0.0
.rodata 101363 101475 112 0.1
.text 553824 553848 24 0.0
pump-app LP_CC2652R7 (read only) 683307 683435 128 0.0
(read/write) 159084 158956 -128 -0.1
.bss 71404 71404 0 0.0
.data 3296 3296 0 0.0
.rodata 89315 89419 104 0.1
.text 593508 593532 24 0.0
pump-controller-app LP_CC2652R7 (read only) 667759 667887 128 0.0
(read/write) 174752 174624 -128 -0.1
.bss 71524 71524 0 0.0
.data 3292 3292 0 0.0
.rodata 84863 84967 104 0.1
.text 582416 582440 24 0.0
shell LP_CC2652R7 (read only) 664018 664154 136 0.0
(read/write) 182892 182756 -136 -0.1
.bss 76604 76604 0 0.0
.data 3376 3376 0 0.0
.rodata 85418 85530 112 0.1
.text 578284 578308 24 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 585118 585126 8 0.0
.app_xip_area 461904 461912 8 0.0
.bss 65656 65656 0 0.0
.data 744 744 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 590918 590926 8 0.0
.app_xip_area 462920 462928 8 0.0
.bss 70440 70440 0 0.0
.data 748 748 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 597466 597602 136 0.0
.app_xip_area 474972 475108 136 0.0
.bss 64968 64968 0 0.0
.data 716 716 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read/write) 1097132 1097276 144 0.0
.bss 132860 132860 0 0.0
.data 2068 2068 0 0.0
.text 962180 962324 144 0.0
BRD4161A+rpc (read/write) 1151412 1151556 144 0.0
.bss 149548 149548 0 0.0
.data 2280 2280 0 0.0
.text 999564 999708 144 0.0
BRD4161A+rs911x (read/write) 986644 986788 144 0.0
.bss 162200 162200 0 0.0
.data 2056 2056 0 0.0
.text 822368 822512 144 0.0
lock-app BRD4161A+wf200 (read/write) 1136980 1137124 144 0.0
.bss 144840 144840 0 0.0
.data 2064 2064 0 0.0
.text 990056 990200 144 0.0
window-app BRD4161A (read/write) 1088584 1088728 144 0.0
.bss 134292 134292 0 0.0
.data 2096 2096 0 0.0
.text 952176 952320 144 0.0
esp32 all-clusters-app c3devkit (read only) 1026616 1026644 28 0.0
(read/write) 1487482 1487594 112 0.0
.dram0.bss 70360 70360 0 0.0
.dram0.data 14600 14600 0 0.0
.flash.rodata 217080 217192 112 0.1
.flash.text 1026616 1026644 28 0.0
.iram0.text 62902 62902 0 0.0
m5stack (read only) 1079907 1079935 28 0.0
(read/write) 489440 489552 112 0.0
.dram0.bss 75864 75864 0 0.0
.dram0.data 34144 34144 0 0.0
.flash.rodata 247436 247548 112 0.0
.flash.text 1074523 1074551 28 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w0+release (read/write) 644076 644076 0 0.0
.bss 69728 69728 0 0.0
.data 2044 2044 0 0.0
.text 569576 569576 0 0.0
lock k32w0+release (read/write) 701364 701364 0 0.0
.bss 70200 70200 0 0.0
.data 2052 2052 0 0.0
.text 626384 626384 0 0.0
linux all-clusters-app debug (read only) 3027977 3028217 240 0.0
(read/write) 155760 155760 0 0.0
.bss 61888 61888 0 0.0
.data 2096 2096 0 0.0
.data.rel.ro 85416 85416 0 0.0
.dynamic 608 608 0 0.0
.got 4568 4568 0 0.0
.init 27 27 0 0.0
.init_array 1160 1160 0 0.0
.rodata 273547 273643 96 0.0
.text 2575474 2575618 144 0.0
all-clusters-minimal-app debug (read only) 2867769 2868041 272 0.0
(read/write) 147496 147496 0 0.0
.bss 61152 61152 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 78008 78008 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 1152 1152 0 0.0
.rodata 273675 273803 128 0.0
.text 2417842 2417986 144 0.0
bridge-app debug+rpc (read only) 2366929 2366929 0 0.0
(read/write) 127456 127456 0 0.0
.bss 50624 50624 0 0.0
.data 3600 3600 0 0.0
.data.rel.ro 67368 67368 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 824 824 0 0.0
.rodata 202600 202600 0 0.0
.text 2001826 2001826 0 0.0
chip-tool debug (read only) 10605713 10605953 240 0.0
(read/write) 648480 648480 0 0.0
.bss 24856 24856 0 0.0
.data 3266 3266 0 0.0
.data.rel.ro 613832 613832 0 0.0
.dynamic 608 608 0 0.0
.got 5104 5104 0 0.0
.init 27 27 0 0.0
.init_array 768 768 0 0.0
.rodata 543989 544085 96 0.0
.text 8560564 8560708 144 0.0
chip-tool-ipv6only arm64 (read only) 10014540 10014796 256 0.0
(read/write) 696209 696209 0 0.0
.bss 32897 32897 0 0.0
.data 3272 3272 0 0.0
.data.rel.ro 641392 641392 0 0.0
.dynamic 560 560 0 0.0
.got 13672 13672 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 474444 474556 112 0.0
.text 7907908 7908052 144 0.0
lighting-app debug+rpc (read only) 2590849 2590945 96 0.0
(read/write) 130048 130048 0 0.0
.bss 49760 49760 0 0.0
.data 2096 2096 0 0.0
.data.rel.ro 72248 72248 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 920 920 0 0.0
.rodata 219280 219376 96 0.0
.text 2200978 2200978 0 0.0
lock-app debug (read only) 2574793 2574921 128 0.0
(read/write) 125416 125416 0 0.0
.bss 48288 48288 0 0.0
.data 1712 1712 0 0.0
.data.rel.ro 69416 69416 0 0.0
.dynamic 608 608 0 0.0
.got 4464 4464 0 0.0
.init 27 27 0 0.0
.init_array 896 896 0 0.0
.rodata 236368 236496 128 0.1
.text 2171890 2171890 0 0.0
ota-provider-app debug (read only) 2352049 2352049 0 0.0
(read/write) 118848 118848 0 0.0
.bss 47808 47808 0 0.0
.data 1936 1936 0 0.0
.data.rel.ro 63224 63224 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 760 760 0 0.0
.rodata 208440 208440 0 0.0
.text 1980258 1980258 0 0.0
ota-requestor-app debug (read only) 2517425 2517681 256 0.0
(read/write) 127192 127192 0 0.0
.bss 50304 50304 0 0.0
.data 2304 2304 0 0.0
.data.rel.ro 68632 68632 0 0.0
.dynamic 608 608 0 0.0
.got 4480 4480 0 0.0
.init 27 27 0 0.0
.init_array 848 848 0 0.0
.rodata 215040 215168 128 0.1
.text 2129938 2130066 128 0.0
shell debug (read only) 2596345 2596601 256 0.0
(read/write) 141912 141912 0 0.0
.bss 57800 57800 0 0.0
.data 1264 1264 0 0.0
.data.rel.ro 77024 77024 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 1032 1032 0 0.0
.rodata 233650 233778 128 0.1
.text 2204466 2204594 128 0.0
thermostat-no-ble arm64 (read only) 2351772 2352012 240 0.0
(read/write) 141681 141681 0 0.0
.bss 55313 55313 0 0.0
.data 1672 1672 0 0.0
.data.rel.ro 75880 75880 0 0.0
.dynamic 560 560 0 0.0
.got 5040 5040 0 0.0
.init 24 24 0 0.0
.init_array 408 408 0 0.0
.rodata 139716 139828 112 0.1
.text 1974528 1974656 128 0.0
tv-app debug (read only) 3170041 3170297 256 0.0
(read/write) 257872 257872 0 0.0
.bss 167480 167480 0 0.0
.data 4736 4736 0 0.0
.data.rel.ro 79096 79096 0 0.0
.dynamic 608 608 0 0.0
.got 4864 4864 0 0.0
.init 27 27 0 0.0
.init_array 1072 1072 0 0.0
.rodata 258248 258376 128 0.0
.text 2722002 2722130 128 0.0
tv-casting-app debug (read only) 5410729 5410985 256 0.0
(read/write) 158992 158992 0 0.0
.bss 51448 51448 0 0.0
.data 2432 2432 0 0.0
.data.rel.ro 98672 98672 0 0.0
.dynamic 608 608 0 0.0
.got 4784 4784 0 0.0
.init 27 27 0 0.0
.init_array 1040 1040 0 0.0
.rodata 341521 341649 128 0.0
.text 4800882 4801010 128 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2452976 2452976 0 0.0
.bss 214572 214572 0 0.0
.data 5872 5872 0 0.0
.text 1415620 1415620 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1178359 1178503 144 0.0
bss 143230 143230 0 0.0
rodata 142776 142888 112 0.1
text 813520 813544 24 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1158383 1158523 140 0.0
bss 142468 142468 0 0.0
rodata 134472 134580 108 0.1
text 802508 802532 24 0.0
p6 all-clusters-app default (read only) 881512 881512 0 0.0
(read/write) 1694476 1694620 144 0.0
.bss 149176 149176 0 0.0
.data 2656 2656 0 0.0
.text 1534256 1534400 144 0.0
all-clusters-minimal-app default (read only) 882232 882232 0 0.0
(read/write) 1638516 1638660 144 0.0
.bss 148456 148456 0 0.0
.data 2656 2656 0 0.0
.text 1479016 1479160 144 0.0
light-app default (read only) 890552 890552 0 0.0
(read/write) 1557348 1557348 0 0.0
.bss 140344 140344 0 0.0
.data 2448 2448 0 0.0
.text 1406168 1406168 0 0.0
lock-app default (read only) 886056 886056 0 0.0
(read/write) 1594516 1594516 0 0.0
.bss 144824 144824 0 0.0
.data 2464 2464 0 0.0
.text 1438840 1438840 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 806472 806616 144 0.0
bss 70976 70976 0 0.0
noinit 43488 43488 0 0.0
text 570178 570206 28 0.0
lighting-app tlsr9518adk80d (read/write) 828440 828576 136 0.0
bss 71832 71832 0 0.0
noinit 43488 43488 0 0.0
text 588294 588320 26 0.0

@bzbarsky-apple
Copy link
Contributor Author

Fast-tracking platform change with review from platform owner and one other company.

@bzbarsky-apple bzbarsky-apple merged commit b5ca125 into project-chip:master Aug 11, 2022
@bzbarsky-apple bzbarsky-apple deleted the fix-timed-invoke-bits branch August 11, 2022 15:01
isiu-apple pushed a commit to isiu-apple/connectedhomeip that referenced this pull request Sep 16, 2022
…ntId. (project-chip#21804)

* Fix CommandSender to error out early if it detects that its "am I a timed
  invoke" state does not match whether a timed invoke timeout was provided.
  This should prevent invalid message sequences from being sent by accident.
* Move the one actual test from MTRClustersTests to MTRDeviceTests.
* Enable running MTRDeviceTests in CI.
* Add a test to MTRDeviceTests that does a timed invoke.
* Fix incorrect initialization of CommandSender in invokeCommandWithEndpointId

The tests being enabled fail without that last item.
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.

3 participants