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 MinInterval handling in ReadClient. #19854

Merged

Conversation

bzbarsky-apple
Copy link
Contributor

We had codepaths on which we did not store MinInterval for an outgoing
subscribe request, and would later report the wrong MinInterval.

Problem

chip-tool subscribe ended up showing minInterval as 0, because we never stored the incoming value on the codepath chip-tool ends up using.

Change overview

Store the value on the codepath all subscribes go through.

Testing

Ran chip-tool onoff subscribe on-off 90 900 17 1 and observed that the log says:

Subscription established with SubscriptionID = 0x42c18ac6 MinInterval = 90s MaxInterval = 900s Peer = 01:0000000000000011

where before it would say:

Subscription established with SubscriptionID = 0xe3d42426 MinInterval = 0s MaxInterval = 900s Peer = 01:0000000000000011

We had codepaths on which we did not store MinInterval for an outgoing
subscribe request, and would later report the wrong MinInterval.
@github-actions
Copy link

github-actions bot commented Jun 22, 2022

PR #19854: Size comparison from d1a4bb7 to d841a01

Increases (9 builds for cc13x2_26x2, efr32, esp32, linux, telink)
platform target config section d1a4bb7 d841a01 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read/write) 191168 191176 8 0.0
lock-ftd LP_CC2652R7 (read/write) 149720 149728 8 0.0
pump-controller-app LP_CC2652R7 (read/write) 183604 183612 8 0.0
shell LP_CC2652R7 (read/write) 156912 156928 16 0.0
efr32 lighting-app BRD4161A+rpc (read only) 963216 963232 16 0.0
.text 963208 963224 16 0.0
esp32 all-clusters-app c3devkit (read only) 1014134 1014136 2 0.0
.flash.text 1014134 1014136 2 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9925180 9925196 16 0.0
.text 7910292 7910308 16 0.0
telink light-switch-app tlsr9518adk80d text 561060 561064 4 0.0
lighting-app tlsr9518adk80d (read/write) 811784 811792 8 0.0
text 577520 577524 4 0.0
Decreases (10 builds for cc13x2_26x2, cyw30739, nrfconnect)
platform target config section d1a4bb7 d841a01 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 660439 660431 -8 -0.0
.text 573152 573144 -8 -0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 649251 649243 -8 -0.0
.text 558424 558416 -8 -0.0
lock-ftd LP_CC2652R7 (read only) 691999 691991 -8 -0.0
.text 592164 592156 -8 -0.0
lock-mtd LP_CC2652R7 (read only) 641415 641407 -8 -0.0
.text 541684 541676 -8 -0.0
pump-controller-app LP_CC2652R7 (read only) 659123 659115 -8 -0.0
.text 575136 575128 -8 -0.0
shell LP_CC2652R7 (read only) 690198 690182 -16 -0.0
.text 580128 580112 -16 -0.0
cyw30739 light cyw930739m2evb_01 (read/write) 591730 591722 -8 -0.0
.app_xip_area 462628 462620 -8 -0.0
lock cyw930739m2evb_01 (read/write) 589106 589098 -8 -0.0
.app_xip_area 459828 459820 -8 -0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 text 823984 823980 -4 -0.0
all-clusters-minimal-app nrf52840dk_nrf52840 text 796032 796028 -4 -0.0
Full report (30 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section d1a4bb7 d841a01 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 660439 660431 -8 -0.0
(read/write) 191168 191176 8 0.0
.bss 74500 74500 0 0.0
.data 3356 3356 0 0.0
.rodata 86975 86975 0 0.0
.text 573152 573144 -8 -0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 649251 649243 -8 -0.0
(read/write) 158060 158060 0 0.0
.bss 73788 73788 0 0.0
.data 3356 3356 0 0.0
.rodata 90507 90507 0 0.0
.text 558424 558416 -8 -0.0
lock-ftd LP_CC2652R7 (read only) 691999 691991 -8 -0.0
(read/write) 149720 149728 8 0.0
.bss 71500 71500 0 0.0
.data 3280 3280 0 0.0
.rodata 99351 99351 0 0.0
.text 592164 592156 -8 -0.0
lock-mtd LP_CC2652R7 (read only) 641415 641407 -8 -0.0
(read/write) 144632 144632 0 0.0
.bss 67236 67236 0 0.0
.data 3280 3280 0 0.0
.rodata 99239 99239 0 0.0
.text 541684 541676 -8 -0.0
pump-app LP_CC2652R7 (read only) 673231 673231 0 0.0
(read/write) 169384 169384 0 0.0
.bss 71628 71628 0 0.0
.data 3280 3280 0 0.0
.rodata 87663 87663 0 0.0
.text 585084 585084 0 0.0
pump-controller-app LP_CC2652R7 (read only) 659123 659115 -8 -0.0
(read/write) 183604 183612 8 0.0
.bss 71740 71740 0 0.0
.data 3276 3276 0 0.0
.rodata 83507 83507 0 0.0
.text 575136 575128 -8 -0.0
shell LP_CC2652R7 (read only) 690198 690182 -16 -0.0
(read/write) 156912 156928 16 0.0
.bss 76804 76804 0 0.0
.data 3360 3360 0 0.0
.rodata 109758 109758 0 0.0
.text 580128 580112 -16 -0.0
cyw30739 light cyw930739m2evb_01 (read/write) 591730 591722 -8 -0.0
.app_xip_area 462628 462620 -8 -0.0
.bss 72064 72064 0 0.0
.data 716 716 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 589106 589098 -8 -0.0
.app_xip_area 459828 459820 -8 -0.0
.bss 72240 72240 0 0.0
.data 720 720 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 594250 594250 0 0.0
.app_xip_area 466020 466020 0 0.0
.bss 71248 71248 0 0.0
.data 660 660 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 926072 926072 0 0.0
(read/write) 133664 133664 0 0.0
.bss 131584 131584 0 0.0
.data 2080 2080 0 0.0
.text 926064 926064 0 0.0
BRD4161A+rpc (read only) 963216 963232 16 0.0
(read/write) 150552 150552 0 0.0
.bss 148264 148264 0 0.0
.data 2284 2284 0 0.0
.text 963208 963224 16 0.0
BRD4161A+rs911x (read only) 802580 802580 0 0.0
(read/write) 140316 140316 0 0.0
.bss 138228 138228 0 0.0
.data 2088 2088 0 0.0
.text 802572 802572 0 0.0
lock-app BRD4161A+wf200 (read only) 968288 968288 0 0.0
(read/write) 140692 140692 0 0.0
.bss 138604 138604 0 0.0
.data 2088 2088 0 0.0
.text 968280 968280 0 0.0
window-app BRD4161A (read only) 911240 911240 0 0.0
(read/write) 133768 133768 0 0.0
.bss 131656 131656 0 0.0
.data 2108 2108 0 0.0
.text 911232 911232 0 0.0
esp32 all-clusters-app c3devkit (read only) 1014134 1014136 2 0.0
(read/write) 1484770 1484770 0 0.0
.dram0.bss 70640 70640 0 0.0
.dram0.data 14632 14632 0 0.0
.flash.rodata 214064 214064 0 0.0
.flash.text 1014134 1014136 2 0.0
.iram0.text 62902 62902 0 0.0
m5stack (read only) 1068459 1068459 0 0.0
(read/write) 486856 486856 0 0.0
.dram0.bss 76152 76152 0 0.0
.dram0.data 34144 34144 0 0.0
.flash.rodata 244564 244564 0 0.0
.flash.text 1063075 1063075 0 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 661524 661524 0 0.0
.bss 69756 69756 0 0.0
.data 1992 1992 0 0.0
.text 583976 583976 0 0.0
lock k32w061+release (read/write) 723452 723452 0 0.0
.bss 70204 70204 0 0.0
.data 2000 2000 0 0.0
.text 645448 645448 0 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9925180 9925196 16 0.0
(read/write) 676161 676161 0 0.0
.bss 42641 42641 0 0.0
.data 1152 1152 0 0.0
.data.rel.ro 615080 615080 0 0.0
.dynamic 528 528 0 0.0
.got 13472 13472 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 473916 473916 0 0.0
.text 7910292 7910308 16 0.0
thermostat-no-ble arm64 (read only) 2578364 2578364 0 0.0
(read/write) 180609 180609 0 0.0
.bss 87905 87905 0 0.0
.data 1704 1704 0 0.0
.data.rel.ro 82896 82896 0 0.0
.dynamic 528 528 0 0.0
.got 5080 5080 0 0.0
.init 24 24 0 0.0
.init_array 400 400 0 0.0
.rodata 164540 164540 0 0.0
.text 2174752 2174752 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2439432 2439432 0 0.0
.bss 212180 212180 0 0.0
.data 5864 5864 0 0.0
.text 1402076 1402076 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1201883 1201883 0 0.0
bss 142862 142862 0 0.0
rodata 156108 156108 0 0.0
text 823984 823980 -4 -0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1149639 1149639 0 0.0
bss 142098 142098 0 0.0
rodata 132592 132592 0 0.0
text 796032 796028 -4 -0.0
p6 all-clusters-app default (read/write) 2555776 2555776 0 0.0
.bss 147360 147360 0 0.0
.data 2776 2776 0 0.0
.text 1514040 1514040 0 0.0
all-clusters-minimal-app default (read/write) 2501624 2501624 0 0.0
.bss 146640 146640 0 0.0
.data 2776 2776 0 0.0
.text 1459888 1459888 0 0.0
light-app default (read/write) 2432384 2432384 0 0.0
.bss 138720 138720 0 0.0
.data 2592 2592 0 0.0
.text 1390648 1390648 0 0.0
lock-app default (read/write) 2453096 2453096 0 0.0
.bss 138544 138544 0 0.0
.data 2600 2600 0 0.0
.text 1411360 1411360 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 792080 792080 0 0.0
bss 71140 71140 0 0.0
noinit 40416 40416 0 0.0
text 561060 561064 4 0.0
lighting-app tlsr9518adk80d (read/write) 811784 811792 8 0.0
bss 71388 71388 0 0.0
noinit 40416 40416 0 0.0
text 577520 577524 4 0.0

@woody-apple woody-apple merged commit 92cf41f into project-chip:master Jun 22, 2022
@bzbarsky-apple bzbarsky-apple deleted the fix-min-interval-storage branch June 22, 2022 20:32
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