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 handling of short discriminator in Linux BLE scan. #21241

Merged

Conversation

woody-apple
Copy link
Contributor

A few changes here:

  1. Implement a SetupDiscriminator class that commons up logic like "does this
    discriminator, which might be short or long, match the given long
    discriminator?" and "extract short discriminator from long discriminator".
  2. Change SetupPayload to more clearly indicate whether it's storing a short or
    long discriminator, instead of storing a short discriminator the same way as
    a long discriminator with the low bits all 0 and hoping consumers check
    isShortDiscriminator.
  3. Update BLE scanning APIs to take SetupDiscriminator.
  4. Fix the Linux and Tizen BLE code to properly handle short discriminators
    (which used to not match if the long discriminator happened to have the low 8
    bits nonzero).
  5. Fix the Darwin BLE code to properly handle long discriminators that have 0
    low bits. Before this change they used to incorrectly match a long
    discriminator which had the same 4 high bits but different 8 low bits.

Fixes #21160

Problem

See above.

Change overview

See above.

Testing

Ensured that BLE commissioning still works on Darwin (which is where I can test) and that chip-tool payload parsing commands work right.

* Fix handling of short discriminator in Linux BLE scan.

A few changes here:

1. Implement a SetupDiscriminator class that commons up logic like "does this
   discriminator, which might be short or long, match the given long
   discriminator?" and "extract short discriminator from long discriminator".
2. Change SetupPayload to more clearly indicate whether it's storing a short or
   long discriminator, instead of storing a short discriminator the same way as
   a long discriminator with the low bits all 0 and hoping consumers check
   isShortDiscriminator.
3. Update BLE scanning APIs to take SetupDiscriminator.
4. Fix the Linux and Tizen BLE code to properly handle short discriminators
   (which used to not match if the long discriminator happened to have the low 8
   bits nonzero).
5. Fix the Darwin BLE code to properly handle long discriminators that have 0
   low bits.  Before this change they used to incorrectly match a long
   discriminator which had the same 4 high bits but different 8 low bits.

Fixes #21160

* Address review comment.
@github-actions
Copy link

github-actions bot commented Jul 26, 2022

PR #21241: Size comparison from e556daa to 9e041aa

Increases above 0.2%:

platform target config section e556daa 9e041aa change % change
linux bridge-app debug+rpc .rodata 199848 200328 480 0.2
chip-tool debug .rodata 531285 532629 1344 0.3
lighting-app debug+rpc .rodata 215472 215952 480 0.2
ota-provider-app debug .rodata 205528 206008 480 0.2
ota-requestor-app debug .rodata 209056 209568 512 0.2
tv-app debug .rodata 251240 252040 800 0.3
tv-casting-app debug .rodata 335361 337281 1920 0.6
Increases (42 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, nrfconnect, p6, telink)
platform target config section e556daa 9e041aa change % change
bl602 lighting-app bl602 (read/write) 1381426 1381530 104 0.0
.text 1051484 1051584 100 0.0
bl602+rpc (read/write) 1426866 1426978 112 0.0
.text 1083168 1083280 112 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 668543 668591 48 0.0
.text 579868 579916 48 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 634127 634183 56 0.0
.text 556220 556276 56 0.0
lock-ftd LP_CC2652R7 (read only) 671523 671563 40 0.0
.text 594600 594640 40 0.0
lock-mtd LP_CC2652R7 (read only) 653807 653847 40 0.0
.text 552152 552192 40 0.0
pump-app LP_CC2652R7 (read only) 681019 681067 48 0.0
.text 591484 591532 48 0.0
pump-controller-app LP_CC2652R7 (read only) 666779 666835 56 0.0
.text 581408 581464 56 0.0
shell LP_CC2652R7 (read only) 661018 661050 32 0.0
.text 575572 575604 32 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 583306 583346 40 0.0
.app_xip_area 460588 460628 40 0.0
lock cyw930739m2evb_01 (read/write) 589226 589258 32 0.0
.app_xip_area 461780 461812 32 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 587494 587534 40 0.0
.app_xip_area 465624 465664 40 0.0
efr32 lighting-app BRD4161A (read/write) 1088352 1088400 48 0.0
.text 953008 953056 48 0.0
BRD4161A+rpc (read/write) 1142684 1142716 32 0.0
.text 990448 990480 32 0.0
BRD4161A+rs911x (read/write) 973544 973592 48 0.0
.text 809748 809796 48 0.0
lock-app BRD4161A+wf200 (read/write) 1128132 1128164 32 0.0
.text 981696 981728 32 0.0
window-app BRD4161A (read/write) 1081844 1081892 48 0.0
.text 944996 945044 48 0.0
esp32 all-clusters-app c3devkit (read only) 1022442 1022526 84 0.0
.flash.text 1022442 1022526 84 0.0
m5stack (read only) 1075947 1076055 108 0.0
.flash.text 1070563 1070671 108 0.0
k32w light k32w0+release (read/write) 641748 641796 48 0.0
.text 567264 567312 48 0.0
lock k32w0+release (read/write) 698820 698868 48 0.0
.text 623888 623936 48 0.0
linux all-clusters-app debug (read only) 2984729 2985801 1072 0.0
.rodata 267467 267979 512 0.2
.text 2539074 2539634 560 0.0
all-clusters-minimal-app debug (read only) 2827625 2828713 1088 0.0
.rodata 267499 268011 512 0.2
.text 2384546 2385122 576 0.0
bridge-app debug+rpc (read only) 2342057 2343113 1056 0.0
.rodata 199848 200328 480 0.2
.text 1979778 1980354 576 0.0
chip-tool debug (read only) 10383473 10385649 2176 0.0
.rodata 531285 532629 1344 0.3
.text 8400708 8401540 832 0.0
chip-tool-ipv6only arm64 (read only) 9808164 9809348 1184 0.0
.rodata 466084 466372 288 0.1
.text 7759876 7760772 896 0.0
lighting-app debug+rpc (read only) 2565145 2566201 1056 0.0
.rodata 215472 215952 480 0.2
.text 2179154 2179730 576 0.0
lock-app debug (read only) 2530185 2531209 1024 0.0
.rodata 230544 230992 448 0.2
.text 2133922 2134498 576 0.0
ota-provider-app debug (read only) 2333961 2335017 1056 0.0
.rodata 205528 206008 480 0.2
.text 1965106 1965682 576 0.0
ota-requestor-app debug (read only) 2452129 2453217 1088 0.0
.rodata 209056 209568 512 0.2
.text 2071346 2071922 576 0.0
shell debug (read only) 2568385 2569249 864 0.0
.rodata 229874 230194 320 0.1
.text 2180834 2181378 544 0.0
thermostat-no-ble arm64 (read only) 2340908 2341596 688 0.0
.rodata 139316 139476 160 0.1
.text 1964688 1965216 528 0.0
tv-app debug (read only) 3115209 3116633 1424 0.0
.rodata 251240 252040 800 0.3
.text 2675426 2676050 624 0.0
tv-casting-app debug (read only) 5369529 5371993 2464 0.0
.rodata 335361 337281 1920 0.6
.text 4767522 4768066 544 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1177471 1177519 48 0.0
text 812872 812920 48 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1157523 1157571 48 0.0
text 802184 802236 52 0.0
p6 all-clusters-app default (read/write) 1687364 1687412 48 0.0
.text 1527200 1527248 48 0.0
all-clusters-minimal-app default (read/write) 1631484 1631548 64 0.0
.text 1472040 1472104 64 0.0
light-app default (read/write) 1551340 1551388 48 0.0
.text 1400200 1400248 48 0.0
lock-app default (read/write) 1588940 1588988 48 0.0
.text 1433328 1433376 48 0.0
telink light-switch-app tlsr9518adk80d (read/write) 799688 799736 48 0.0
text 567360 567404 44 0.0
lighting-app tlsr9518adk80d (read/write) 819772 819820 48 0.0
text 583912 583956 44 0.0
Decreases (6 builds for cc13x2_26x2)
platform target config section e556daa 9e041aa change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read/write) 182816 182768 -48 -0.0
lock-ftd LP_CC2652R7 (read/write) 170028 169988 -40 -0.0
lock-mtd LP_CC2652R7 (read/write) 183432 183392 -40 -0.0
pump-app LP_CC2652R7 (read/write) 161364 161316 -48 -0.0
pump-controller-app LP_CC2652R7 (read/write) 175740 175684 -56 -0.0
shell LP_CC2652R7 (read/write) 185860 185828 -32 -0.0
Full report (43 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section e556daa 9e041aa change % change
bl602 lighting-app bl602 (read/write) 1381426 1381530 104 0.0
.bss 117538 117538 0 0.0
.data 4480 4480 0 0.0
.text 1051484 1051584 100 0.0
bl602+rpc (read/write) 1426866 1426978 112 0.0
.bss 124978 124978 0 0.0
.data 4600 4600 0 0.0
.text 1083168 1083280 112 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 668543 668591 48 0.0
(read/write) 182816 182768 -48 -0.0
.bss 74252 74252 0 0.0
.data 3356 3356 0 0.0
.rodata 88359 88359 0 0.0
.text 579868 579916 48 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 634127 634183 56 0.0
(read/write) 157820 157820 0 0.0
.bss 73548 73548 0 0.0
.data 3356 3356 0 0.0
.rodata 77583 77583 0 0.0
.text 556220 556276 56 0.0
lock-ftd LP_CC2652R7 (read only) 671523 671563 40 0.0
(read/write) 170028 169988 -40 -0.0
.bss 71332 71332 0 0.0
.data 3280 3280 0 0.0
.rodata 76443 76443 0 0.0
.text 594600 594640 40 0.0
lock-mtd LP_CC2652R7 (read only) 653807 653847 40 0.0
(read/write) 183432 183392 -40 -0.0
.bss 67020 67020 0 0.0
.data 3280 3280 0 0.0
.rodata 101175 101175 0 0.0
.text 552152 552192 40 0.0
pump-app LP_CC2652R7 (read only) 681019 681067 48 0.0
(read/write) 161364 161316 -48 -0.0
.bss 71396 71396 0 0.0
.data 3280 3280 0 0.0
.rodata 89051 89051 0 0.0
.text 591484 591532 48 0.0
pump-controller-app LP_CC2652R7 (read only) 666779 666835 56 0.0
(read/write) 175740 175684 -56 -0.0
.bss 71532 71532 0 0.0
.data 3276 3276 0 0.0
.rodata 84891 84891 0 0.0
.text 581408 581464 56 0.0
shell LP_CC2652R7 (read only) 661018 661050 32 0.0
(read/write) 185860 185828 -32 -0.0
.bss 76572 76572 0 0.0
.data 3360 3360 0 0.0
.rodata 85130 85130 0 0.0
.text 575572 575604 32 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 583306 583346 40 0.0
.app_xip_area 460588 460628 40 0.0
.bss 65656 65656 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) 589226 589258 32 0.0
.app_xip_area 461780 461812 32 0.0
.bss 70384 70384 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) 587494 587534 40 0.0
.app_xip_area 465624 465664 40 0.0
.bss 64864 64864 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/write) 1088352 1088400 48 0.0
.bss 133276 133276 0 0.0
.data 2048 2048 0 0.0
.text 953008 953056 48 0.0
BRD4161A+rpc (read/write) 1142684 1142716 32 0.0
.bss 149956 149956 0 0.0
.data 2260 2260 0 0.0
.text 990448 990480 32 0.0
BRD4161A+rs911x (read/write) 973544 973592 48 0.0
.bss 161728 161728 0 0.0
.data 2048 2048 0 0.0
.text 809748 809796 48 0.0
lock-app BRD4161A+wf200 (read/write) 1128132 1128164 32 0.0
.bss 144360 144360 0 0.0
.data 2056 2056 0 0.0
.text 981696 981728 32 0.0
window-app BRD4161A (read/write) 1081844 1081892 48 0.0
.bss 134748 134748 0 0.0
.data 2076 2076 0 0.0
.text 944996 945044 48 0.0
esp32 all-clusters-app c3devkit (read only) 1022442 1022526 84 0.0
(read/write) 1486442 1486442 0 0.0
.dram0.bss 70288 70288 0 0.0
.dram0.data 14600 14600 0 0.0
.flash.rodata 216120 216120 0 0.0
.flash.text 1022442 1022526 84 0.0
.iram0.text 62902 62902 0 0.0
m5stack (read only) 1075947 1076055 108 0.0
(read/write) 488464 488464 0 0.0
.dram0.bss 75800 75800 0 0.0
.dram0.data 34144 34144 0 0.0
.flash.rodata 246524 246524 0 0.0
.flash.text 1070563 1070671 108 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w0+release (read/write) 641748 641796 48 0.0
.bss 69728 69728 0 0.0
.data 2028 2028 0 0.0
.text 567264 567312 48 0.0
lock k32w0+release (read/write) 698820 698868 48 0.0
.bss 70168 70168 0 0.0
.data 2036 2036 0 0.0
.text 623888 623936 48 0.0
linux all-clusters-app debug (read only) 2984729 2985801 1072 0.0
(read/write) 155416 155416 0 0.0
.bss 61856 61856 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 85224 85224 0 0.0
.dynamic 608 608 0 0.0
.got 4568 4568 0 0.0
.init 27 27 0 0.0
.init_array 1072 1072 0 0.0
.rodata 267467 267979 512 0.2
.text 2539074 2539634 560 0.0
all-clusters-minimal-app debug (read only) 2827625 2828713 1088 0.0
(read/write) 147120 147120 0 0.0
.bss 61056 61056 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 77816 77816 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 1064 1064 0 0.0
.rodata 267499 268011 512 0.2
.text 2384546 2385122 576 0.0
bridge-app debug+rpc (read only) 2342057 2343113 1056 0.0
(read/write) 127024 127024 0 0.0
.bss 50144 50144 0 0.0
.data 3824 3824 0 0.0
.data.rel.ro 67272 67272 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 744 744 0 0.0
.rodata 199848 200328 480 0.2
.text 1979778 1980354 576 0.0
chip-tool debug (read only) 10383473 10385649 2176 0.0
(read/write) 631056 631056 0 0.0
.bss 24824 24824 0 0.0
.data 3266 3266 0 0.0
.data.rel.ro 596568 596568 0 0.0
.dynamic 608 608 0 0.0
.got 5088 5088 0 0.0
.init 27 27 0 0.0
.init_array 656 656 0 0.0
.rodata 531285 532629 1344 0.3
.text 8400708 8401540 832 0.0
chip-tool-ipv6only arm64 (read only) 9808164 9809348 1184 0.0
(read/write) 678561 678561 0 0.0
.bss 32897 32897 0 0.0
.data 3272 3272 0 0.0
.data.rel.ro 623904 623904 0 0.0
.dynamic 560 560 0 0.0
.got 13536 13536 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 466084 466372 288 0.1
.text 7759876 7760772 896 0.0
lighting-app debug+rpc (read only) 2565145 2566201 1056 0.0
(read/write) 129992 129992 0 0.0
.bss 49696 49696 0 0.0
.data 2096 2096 0 0.0
.data.rel.ro 72328 72328 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 832 832 0 0.0
.rodata 215472 215952 480 0.2
.text 2179154 2179730 576 0.0
lock-app debug (read only) 2530185 2531209 1024 0.0
(read/write) 124976 124976 0 0.0
.bss 48096 48096 0 0.0
.data 1712 1712 0 0.0
.data.rel.ro 69304 69304 0 0.0
.dynamic 608 608 0 0.0
.got 4424 4424 0 0.0
.init 27 27 0 0.0
.init_array 808 808 0 0.0
.rodata 230544 230992 448 0.2
.text 2133922 2134498 576 0.0
ota-provider-app debug (read only) 2333961 2335017 1056 0.0
(read/write) 118776 118776 0 0.0
.bss 47744 47744 0 0.0
.data 1936 1936 0 0.0
.data.rel.ro 63288 63288 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 688 688 0 0.0
.rodata 205528 206008 480 0.2
.text 1965106 1965682 576 0.0
ota-requestor-app debug (read only) 2452129 2453217 1088 0.0
(read/write) 125680 125680 0 0.0
.bss 50080 50080 0 0.0
.data 2240 2240 0 0.0
.data.rel.ro 67512 67512 0 0.0
.dynamic 608 608 0 0.0
.got 4480 4480 0 0.0
.init 27 27 0 0.0
.init_array 744 744 0 0.0
.rodata 209056 209568 512 0.2
.text 2071346 2071922 576 0.0
shell debug (read only) 2568385 2569249 864 0.0
(read/write) 141568 141568 0 0.0
.bss 57704 57704 0 0.0
.data 1264 1264 0 0.0
.data.rel.ro 76888 76888 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 944 944 0 0.0
.rodata 229874 230194 320 0.1
.text 2180834 2181378 544 0.0
thermostat-no-ble arm64 (read only) 2340908 2341596 688 0.0
(read/write) 141345 141345 0 0.0
.bss 55297 55297 0 0.0
.data 1672 1672 0 0.0
.data.rel.ro 75624 75624 0 0.0
.dynamic 560 560 0 0.0
.got 4984 4984 0 0.0
.init 24 24 0 0.0
.init_array 400 400 0 0.0
.rodata 139316 139476 160 0.1
.text 1964688 1965216 528 0.0
tv-app debug (read only) 3115209 3116633 1424 0.0
(read/write) 257160 257160 0 0.0
.bss 167160 167160 0 0.0
.data 4736 4736 0 0.0
.data.rel.ro 78824 78824 0 0.0
.dynamic 608 608 0 0.0
.got 4848 4848 0 0.0
.init 27 27 0 0.0
.init_array 968 968 0 0.0
.rodata 251240 252040 800 0.3
.text 2675426 2676050 624 0.0
tv-casting-app debug (read only) 5369529 5371993 2464 0.0
(read/write) 158432 158432 0 0.0
.bss 51320 51320 0 0.0
.data 2432 2432 0 0.0
.data.rel.ro 98384 98384 0 0.0
.dynamic 608 608 0 0.0
.got 4736 4736 0 0.0
.init 27 27 0 0.0
.init_array 928 928 0 0.0
.rodata 335361 337281 1920 0.6
.text 4767522 4768066 544 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2449168 2449168 0 0.0
.bss 214508 214508 0 0.0
.data 5872 5872 0 0.0
.text 1411812 1411812 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1177471 1177519 48 0.0
bss 143132 143132 0 0.0
rodata 142536 142536 0 0.0
text 812872 812920 48 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1157523 1157571 48 0.0
bss 142368 142368 0 0.0
rodata 134068 134068 0 0.0
text 802184 802236 52 0.0
p6 all-clusters-app default (read only) 881568 881568 0 0.0
(read/write) 1687364 1687412 48 0.0
.bss 149128 149128 0 0.0
.data 2648 2648 0 0.0
.text 1527200 1527248 48 0.0
all-clusters-minimal-app default (read only) 882288 882288 0 0.0
(read/write) 1631484 1631548 64 0.0
.bss 148408 148408 0 0.0
.data 2648 2648 0 0.0
.text 1472040 1472104 64 0.0
light-app default (read only) 890592 890592 0 0.0
(read/write) 1551340 1551388 48 0.0
.bss 140312 140312 0 0.0
.data 2440 2440 0 0.0
.text 1400200 1400248 48 0.0
lock-app default (read only) 886120 886120 0 0.0
(read/write) 1588940 1588988 48 0.0
.bss 144768 144768 0 0.0
.data 2456 2456 0 0.0
.text 1433328 1433376 48 0.0
telink light-switch-app tlsr9518adk80d (read/write) 799688 799736 48 0.0
bss 70808 70808 0 0.0
noinit 40416 40416 0 0.0
text 567360 567404 44 0.0
lighting-app tlsr9518adk80d (read/write) 819772 819820 48 0.0
bss 71652 71652 0 0.0
noinit 40416 40416 0 0.0
text 583912 583956 44 0.0

@woody-apple woody-apple merged commit bc36684 into sve Jul 27, 2022
@woody-apple woody-apple deleted the cherry-pick-79e8c9edb805d7d8cf2f529e66fbf9d1c0935629 branch July 27, 2022 16:16
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.

2 participants