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 active mode (fast-poll) management in CommissioningWindowManager. #23298

Conversation

bzbarsky-apple
Copy link
Contributor

Without this fix we could end up in the following situation:

  1. Open a commissioning window when operational. Enter active mode in StartAdvertisement.
  2. Establish a PASE session. Leave active mode in StopAdvertisement.
  3. Commissioning fails, fail-safe triggers.
  4. We call StartAdvertisement, but do not enter active mode, because IsCommissioningWindowOpen() is true at this point.

The fix is to ensure that our RequestSEDActiveMode() calls happen exacty when mListeningForPASE chages value, since that's the reason we want to be messing with active mode to start with.

Without this fix we could end up in the following situation:

1. Open a commissioning window when operational.  Enter active mode in
   StartAdvertisement.
2. Establish a PASE session.  Leave active mode in StopAdvertisement.
3. Commissioning fails, fail-safe triggers.
4. We call StartAdvertisement, but do not enter active mode, because
   IsCommissioningWindowOpen() is true at this point.

The fix is to ensure that our RequestSEDActiveMode() calls happen
exacty when mListeningForPASE chages value, since that's the reason we
want to be messing with active mode to start with.
@github-actions github-actions bot added the app label Oct 21, 2022
@github-actions
Copy link

github-actions bot commented Oct 21, 2022

PR #23298: Size comparison from a325034 to ed41b83

Increases (15 builds for bl602, bl702, cc13x2_26x2, psoc6, qpg, telink)
platform target config section a325034 ed41b83 change % change
bl602 lighting-app bl602 .text 1069636 1069638 2 0.0
bl602+rpc (read/write) 1435462 1435470 8 0.0
.text 1100984 1100988 4 0.0
bl702 lighting-app bl702 .debug_abbrev 1506978 1506996 18 0.0
.debug_info 37909063 37909070 7 0.0
bl702+rpc .debug_abbrev 1644523 1644541 18 0.0
.debug_info 41815676 41815684 8 0.0
.text 1031022 1031024 2 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read/write) 174200 174208 8 0.0
pump-controller-app LP_CC2652R7 (read/write) 177072 177080 8 0.0
psoc6 all-clusters cy8ckit_062s2_43012 .debug_info 26837501 26837512 11 0.0
.debug_line 3673331 3673332 1 0.0
all-clusters-minimal cy8ckit_062s2_43012 .debug_info 26574283 26574293 10 0.0
.debug_line 3694048 3694049 1 0.0
light cy8ckit_062s2_43012 .debug_info 22038005 22038015 10 0.0
.debug_line 3263972 3263973 1 0.0
lock cy8ckit_062s2_43012 .debug_info 22271296 22271308 12 0.0
.debug_line 3260937 3260938 1 0.0
qpg lighting-app qpg6105+debug (read/write) 1148816 1148824 8 0.0
.text 595916 595924 8 0.0
lock-app qpg6105+debug (read/write) 1113704 1113712 8 0.0
.text 560800 560808 8 0.0
telink light-switch-app tlsr9518adk80d text 553858 553860 2 0.0
lighting-app tlsr9518adk80d text 565992 565996 4 0.0
ota-requestor-app tlsr9518adk80d (read/write) 804416 804424 8 0.0
text 564862 564866 4 0.0
Decreases (6 builds for cc13x2_26x2, cyw30739, esp32, k32w)
platform target config section a325034 ed41b83 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 677359 677351 -8 -0.0
.text 587256 587248 -8 -0.0
pump-controller-app LP_CC2652R7 (read only) 672455 672447 -8 -0.0
.text 585656 585648 -8 -0.0
cyw30739 lock cyw930739m2evb_01 (read/write) 592178 592170 -8 -0.0
.app_xip_area 463468 463460 -8 -0.0
esp32 all-clusters-app c3devkit (read only) 1223652 1223648 -4 -0.0
(read/write) 1788342 1788334 -8 -0.0
.flash.rodata 257896 257888 -8 -0.0
.flash.text 1223652 1223648 -4 -0.0
k32w light k32w0+release (read/write) 671528 671512 -16 -0.0
.text 591900 591884 -16 -0.0
lock k32w0+release (read/write) 633092 633060 -32 -0.0
.text 552684 552652 -32 -0.0
Full report (38 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
platform target config section a325034 ed41b83 change % change
bl602 lighting-app bl602 (read/write) 1390246 1390246 0 0.0
.bss 90745 90745 0 0.0
.data 9928 9928 0 0.0
.text 1069636 1069638 2 0.0
bl602+rpc (read/write) 1435462 1435470 8 0.0
.bss 98177 98177 0 0.0
.data 10312 10312 0 0.0
.text 1100984 1100988 4 0.0
bl702 lighting-app bl702 (read only) 3262 3262 0 0.0
(read/write) 1189219 1189219 0 0.0
.bleromro 6296 6296 0 0.0
.bleromrw 124 124 0 0.0
.boot2 688 688 0 0.0
.bss 67118 67118 0 0.0
.bss_psram 29696 29696 0 0.0
.comment 48 48 0 0.0
.data 4272 4272 0 0.0
.debug_abbrev 1506978 1506996 18 0.0
.debug_aranges 133168 133168 0 0.0
.debug_frame 486752 486752 0 0.0
.debug_info 37909063 37909070 7 0.0
.debug_line 5257986 5257986 0 0.0
.debug_loc 3367729 3367729 0 0.0
.debug_ranges 359568 359568 0 0.0
.debug_str 3458100 3458100 0 0.0
.hbn 509 509 0 0.0
.hbn_noinit 260 260 0 0.0
.init 342 342 0 0.0
.init_array 144 144 0 0.0
.psram 0 0 0 0.0
.riscv.attributes 47 47 0 0.0
.rodata 116744 116744 0 0.0
.rsvd 3188 3188 0 0.0
.shstrtab 293 293 0 0.0
.stack 2048 2048 0 0.0
.strtab 565314 565314 0 0.0
.symtab 171728 171728 0 0.0
.tcm_data 36 36 0 0.0
.tcmcode 3262 3262 0 0.0
.text 0 0 0 0.0
957578 957578 0 0.0
bl702+rpc (read only) 3262 3262 0 0.0
(read/write) 1284883 1284883 0 0.0
.bleromro 6296 6296 0 0.0
.bleromrw 124 124 0 0.0
.boot2 688 688 0 0.0
.bss 75166 75166 0 0.0
.bss_psram 29936 29936 0 0.0
.comment 48 48 0 0.0
.data 4800 4800 0 0.0
.debug_abbrev 1644523 1644541 18 0.0
.debug_aranges 140672 140672 0 0.0
.debug_frame 512124 512124 0 0.0
.debug_info 41815676 41815684 8 0.0
.debug_line 5632517 5632517 0 0.0
.debug_loc 3560376 3560376 0 0.0
.debug_ranges 382024 382024 0 0.0
.debug_str 3854070 3854070 0 0.0
.hbn 509 509 0 0.0
.hbn_noinit 260 260 0 0.0
.init 342 342 0 0.0
.init_array 160 160 0 0.0
.psram 0 0 0 0.0
.riscv.attributes 47 47 0 0.0
.rodata 130136 130136 0 0.0
.rsvd 3188 3188 0 0.0
.shstrtab 293 293 0 0.0
.stack 2048 2048 0 0.0
.strtab 624487 624487 0 0.0
.symtab 189536 189536 0 0.0
.tcm_data 36 36 0 0.0
.tcmcode 3262 3262 0 0.0
.text 0 0 0 0.0
1031022 1031024 2 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 677359 677351 -8 -0.0
(read/write) 174200 174208 8 0.0
.bss 81252 81252 0 0.0
.data 3380 3380 0 0.0
.rodata 89791 89791 0 0.0
.text 587256 587248 -8 -0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 641599 641599 0 0.0
(read/write) 158020 158020 0 0.0
.bss 80524 80524 0 0.0
.data 3380 3380 0 0.0
.rodata 78927 78927 0 0.0
.text 562352 562352 0 0.0
lock-ftd LP_CC2652R7 (read only) 675851 675851 0 0.0
(read/write) 172844 172844 0 0.0
.bss 78476 78476 0 0.0
.data 3304 3304 0 0.0
.rodata 77315 77315 0 0.0
.text 598056 598056 0 0.0
lock-mtd LP_CC2652R7 (read only) 659615 659615 0 0.0
(read/write) 184768 184768 0 0.0
.bss 74164 74164 0 0.0
.data 3304 3304 0 0.0
.rodata 103135 103135 0 0.0
.text 556000 556000 0 0.0
pump-app LP_CC2652R7 (read only) 688131 688131 0 0.0
(read/write) 161300 161300 0 0.0
.bss 78444 78444 0 0.0
.data 3296 3296 0 0.0
.rodata 90763 90763 0 0.0
.text 596884 596884 0 0.0
pump-controller-app LP_CC2652R7 (read only) 672455 672447 -8 -0.0
(read/write) 177072 177080 8 0.0
.bss 78540 78540 0 0.0
.data 3292 3292 0 0.0
.rodata 86319 86319 0 0.0
.text 585656 585648 -8 -0.0
shell LP_CC2652R7 (read only) 668382 668382 0 0.0
(read/write) 185496 185496 0 0.0
.bss 83572 83572 0 0.0
.data 3376 3376 0 0.0
.rodata 86502 86502 0 0.0
.text 581564 581564 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 588138 588138 0 0.0
.app_xip_area 464724 464724 0 0.0
.bss 65832 65832 0 0.0
.data 760 760 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 592178 592170 -8 -0.0
.app_xip_area 463468 463460 -8 -0.0
.bss 71120 71120 0 0.0
.data 768 768 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 543998 543998 0 0.0
.app_xip_area 425664 425664 0 0.0
.bss 60800 60800 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+rpc (read/write) 974092 974092 0 0.0
.bss 152276 152276 0 0.0
.data 2248 2248 0 0.0
.text 819548 819548 0 0.0
BRD4161A+rs911x (read/write) 1031640 1031640 0 0.0
.bss 186680 186680 0 0.0
.data 2092 2092 0 0.0
.text 842848 842848 0 0.0
BRD4187C (read/write) 1146348 1146348 0 0.0
.bss 138664 138664 0 0.0
.data 2596 2596 0 0.0
.text 980492 980492 0 0.0
lock-app BRD4161A+wf200 (read/write) 1157248 1157248 0 0.0
.bss 158208 158208 0 0.0
.data 2100 2100 0 0.0
.text 996920 996920 0 0.0
window-app BRD4187C (read/write) 1139568 1139568 0 0.0
.bss 140080 140080 0 0.0
.data 2620 2620 0 0.0
.text 972272 972272 0 0.0
esp32 all-clusters-app c3devkit (read only) 1223652 1223648 -4 -0.0
(read/write) 1788342 1788334 -8 -0.0
.dram0.bss 76960 76960 0 0.0
.dram0.data 13840 13840 0 0.0
.flash.rodata 257896 257888 -8 -0.0
.flash.text 1223652 1223648 -4 -0.0
.iram0.text 65204 65204 0 0.0
m5stack (read only) 1233699 1233699 0 0.0
(read/write) 564252 564252 0 0.0
.dram0.bss 82336 82336 0 0.0
.dram0.data 34296 34296 0 0.0
.flash.rodata 314952 314952 0 0.0
.flash.text 1228315 1228315 0 0.0
.iram0.text 123939 123939 0 0.0
k32w contact k32w0+release (read/write) 661268 661268 0 0.0
.bss 77040 77040 0 0.0
.data 2104 2104 0 0.0
.text 563012 563012 0 0.0
light k32w0+release (read/write) 671528 671512 -16 -0.0
.bss 74840 74840 0 0.0
.data 2060 2060 0 0.0
.text 591900 591884 -16 -0.0
lock k32w0+release (read/write) 633092 633060 -32 -0.0
.bss 75600 75600 0 0.0
.data 2080 2080 0 0.0
.text 552684 552652 -32 -0.0
linux chip-tool-ipv6only arm64 (read only) 10429700 10429700 0 0.0
(read/write) 706353 706353 0 0.0
.bss 33953 33953 0 0.0
.data 2768 2768 0 0.0
.data.rel.ro 650632 650632 0 0.0
.dynamic 560 560 0 0.0
.got 13896 13896 0 0.0
.init 24 24 0 0.0
.init_array 208 208 0 0.0
.rodata 518148 518148 0 0.0
.text 8255988 8255988 0 0.0
thermostat-no-ble arm64 (read only) 2390796 2390796 0 0.0
(read/write) 143633 143633 0 0.0
.bss 55377 55377 0 0.0
.data 1816 1816 0 0.0
.data.rel.ro 77256 77256 0 0.0
.dynamic 560 560 0 0.0
.got 5176 5176 0 0.0
.init 24 24 0 0.0
.init_array 440 440 0 0.0
.rodata 144484 144484 0 0.0
.text 2003408 2003408 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2452200 2452200 0 0.0
.bss 215028 215028 0 0.0
.data 5872 5872 0 0.0
.text 1414844 1414844 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1183659 1183659 0 0.0
bss 144457 144457 0 0.0
rodata 144428 144428 0 0.0
text 815888 815888 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1162343 1162343 0 0.0
bss 143684 143684 0 0.0
rodata 136000 136000 0 0.0
text 803784 803784 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 (read only) 841952 841952 0 0.0
(read/write) 1745644 1745644 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 188728 188728 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2664 2664 0 0.0
.debug_abbrev 1229487 1229487 0 0.0
.debug_aranges 111904 111904 0 0.0
.debug_frame 373628 373628 0 0.0
.debug_info 26837501 26837512 11 0.0
.debug_line 3673331 3673332 1 0.0
.debug_loc 3590250 3590250 0 0.0
.debug_ranges 339704 339704 0 0.0
.debug_str 3441503 3441503 0 0.0
.heap 841952 841952 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 569882 569882 0 0.0
.symtab 421328 421328 0 0.0
.text 1545864 1545864 0 0.0
.zero.table 8 8 0 0.0
text 0 0 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 (read only) 842688 842688 0 0.0
(read/write) 1688244 1688244 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 187992 187992 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2664 2664 0 0.0
.debug_abbrev 1221286 1221286 0 0.0
.debug_aranges 111376 111376 0 0.0
.debug_frame 376708 376708 0 0.0
.debug_info 26574283 26574293 10 0.0
.debug_line 3694048 3694049 1 0.0
.debug_loc 3577887 3577887 0 0.0
.debug_ranges 338320 338320 0 0.0
.debug_str 3430516 3430516 0 0.0
.heap 842688 842688 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 533971 533971 0 0.0
.symtab 407760 407760 0 0.0
.text 1489200 1489200 0 0.0
.zero.table 0 0 0 0.0
8 8 0 0.0
light cy8ckit_062s2_43012 (read only) 850872 850872 0 0.0
(read/write) 1606788 1606788 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 180016 180016 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2456 2456 0 0.0
.debug_abbrev 1055397 1055397 0 0.0
.debug_aranges 103584 103584 0 0.0
.debug_frame 347040 347040 0 0.0
.debug_info 22038005 22038015 10 0.0
.debug_line 3263972 3263973 1 0.0
.debug_loc 3275848 3275848 0 0.0
.debug_ranges 303624 303624 0 0.0
.debug_str 3236048 3236048 0 0.0
.heap 850872 850872 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 470348 470348 0 0.0
.symtab 376208 376208 0 0.0
.text 1415928 1415928 0 0.0
.zero.table 0 0 0 0.0
8 8 0 0.0
lock cy8ckit_062s2_43012 (read only) 845880 845880 0 0.0
(read/write) 1640492 1640492 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 184992 184992 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2472 2472 0 0.0
.debug_abbrev 1057336 1057336 0 0.0
.debug_aranges 103976 103976 0 0.0
.debug_frame 348896 348896 0 0.0
.debug_info 22271296 22271308 12 0.0
.debug_line 3260937 3260938 1 0.0
.debug_loc 3303430 3303430 0 0.0
.debug_ranges 305560 305560 0 0.0
.debug_str 3255446 3255446 0 0.0
.heap 845880 845880 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 472699 472699 0 0.0
.symtab 377824 377824 0 0.0
.text 1444640 1444640 0 0.0
.zero.table 0 0 0 0.0
8 8 0 0.0
qpg lighting-app qpg6105+debug (read/write) 1148816 1148824 8 0.0
.bss 110580 110580 0 0.0
.data 832 832 0 0.0
.text 595916 595924 8 0.0
lock-app qpg6105+debug (read/write) 1113704 1113712 8 0.0
.bss 106372 106372 0 0.0
.data 836 836 0 0.0
.text 560800 560808 8 0.0
telink light-switch-app tlsr9518adk80d (read/write) 790300 790300 0 0.0
bss 72480 72480 0 0.0
noinit 43520 43520 0 0.0
text 553858 553860 2 0.0
lighting-app tlsr9518adk80d (read/write) 805532 805532 0 0.0
bss 73240 73240 0 0.0
noinit 43520 43520 0 0.0
text 565992 565996 4 0.0
ota-requestor-app tlsr9518adk80d (read/write) 804416 804424 8 0.0
bss 74052 74052 0 0.0
noinit 43520 43520 0 0.0
text 564862 564866 4 0.0

@bzbarsky-apple bzbarsky-apple merged commit 8b30418 into project-chip:master Oct 25, 2022
@bzbarsky-apple bzbarsky-apple deleted the safer-fast-mode-commissioning-window-management branch October 25, 2022 12:44
adbridge pushed a commit to ARM-software/connectedhomeip that referenced this pull request Nov 18, 2022
…project-chip#23298)

Without this fix we could end up in the following situation:

1. Open a commissioning window when operational.  Enter active mode in
   StartAdvertisement.
2. Establish a PASE session.  Leave active mode in StopAdvertisement.
3. Commissioning fails, fail-safe triggers.
4. We call StartAdvertisement, but do not enter active mode, because
   IsCommissioningWindowOpen() is true at this point.

The fix is to ensure that our RequestSEDActiveMode() calls happen
exacty when mListeningForPASE chages value, since that's the reason we
want to be messing with active mode to start with.
adbridge pushed a commit to ARM-software/connectedhomeip that referenced this pull request Nov 18, 2022
…project-chip#23298)

Without this fix we could end up in the following situation:

1. Open a commissioning window when operational.  Enter active mode in
   StartAdvertisement.
2. Establish a PASE session.  Leave active mode in StopAdvertisement.
3. Commissioning fails, fail-safe triggers.
4. We call StartAdvertisement, but do not enter active mode, because
   IsCommissioningWindowOpen() is true at this point.

The fix is to ensure that our RequestSEDActiveMode() calls happen
exacty when mListeningForPASE chages value, since that's the reason we
want to be messing with active mode to start with.
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