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.

Sign up for GitHub

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

Jump to bottom

Don't try to do something with message delivery failures on other fabrics. #16157

Merged
merged 2 commits into from
Mar 14, 2022
Merged

Don't try to do something with message delivery failures on other fabrics. #16157

merged 2 commits into from
Mar 14, 2022

Conversation

bzbarsky-apple
Copy link
Contributor

If we have DeviceController instances for multiple fabrics around, all
will be notified via OnFirstMessageDeliveryFailed on message delivery
failure for any message. Ignore the notifications unless they are for
a message over a CASE session for the fabric the controller is
associated with (in which case it might actually make sense for that
controller to act on the notification by redoing operational address
discovery and CASE).

Problem

See above.

Change overview

See above.

Testing

Made sure logs don't show multiple re-resolve attempts on different fabrics when a message is lost.

@bzbarsky-apple
Don't try to do something with message delivery failures on other fab…
c235545 
…rics.

If we have DeviceController instances for multiple fabrics around, all
will be notified via OnFirstMessageDeliveryFailed on message delivery
failure for any message.  Ignore the notifications unless they are for
a message over a CASE session for the fabric the controller is
associated with (in which case it might actually make sense for that
controller to act on the notification by redoing operational address
discovery and CASE).
@github-actions
Copy link

github-actions bot commented Mar 12, 2022 •
edited
Loading

PR #16157: Size comparison from 4638993 to c235545

Increases (1 build for linux)
platform target config section 4638993 c235545 change % change
linux chip-tool-ipv6only arm64 (read only) 9727788 9727884 96 0.0
.text 8185700 8185844 144 0.0
Decreases (1 build for linux)
platform target config section 4638993 c235545 change % change
linux chip-tool-ipv6only arm64 .rodata 492244 492196 -48 -0.0
Full report (17 builds for cyw30739, efr32, esp32, k32w, linux, nrfconnect, p6, telink)
platform target config section 4638993 c235545 change % change
cyw30739 light cyw930739m2evb_01 (read/write) 602530 602530 0 0.0
.app_xip_area 509708 509708 0 0.0
.bss 75576 75576 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) 560390 560390 0 0.0
.app_xip_area 469096 469096 0 0.0
.bss 74080 74080 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) 570002 570002 0 0.0
.app_xip_area 469060 469060 0 0.0
.bss 83384 83384 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) 918144 918144 0 0.0
(read/write) 128656 128656 0 0.0
.bss 126648 126648 0 0.0
.data 2008 2008 0 0.0
.text 918136 918136 0 0.0
BRD4161A+rpc (read only) 946932 946932 0 0.0
(read/write) 144612 144612 0 0.0
.bss 142424 142424 0 0.0
.data 2188 2188 0 0.0
.text 946924 946924 0 0.0
window-app BRD4161A (read only) 850616 850616 0 0.0
(read/write) 126632 126632 0 0.0
.bss 124768 124768 0 0.0
.data 1864 1864 0 0.0
.text 850608 850608 0 0.0
esp32 all-clusters-app c3devkit (read only) 961344 961344 0 0.0
(read/write) 1394994 1394994 0 0.0
.dram0.bss 64056 64056 0 0.0
.dram0.data 14188 14188 0 0.0
.flash.rodata 197832 197832 0 0.0
.flash.text 961344 961344 0 0.0
.iram0.text 62016 62016 0 0.0
m5stack (read only) 1016963 1016963 0 0.0
(read/write) 462204 462204 0 0.0
.dram0.bss 69576 69576 0 0.0
.dram0.data 34016 34016 0 0.0
.flash.rodata 226776 226776 0 0.0
.flash.text 1011579 1011579 0 0.0
.iram0.text 123107 123107 0 0.0
k32w light k32w061+release (read/write) 699052 699052 0 0.0
.bss 77560 77560 0 0.0
.data 1888 1888 0 0.0
.text 613804 613804 0 0.0
lock k32w061+release (read/write) 699884 699884 0 0.0
.bss 77552 77552 0 0.0
.data 1908 1908 0 0.0
.text 614624 614624 0 0.0
linux chip-tool-ipv6only arm64 (read only) 9727788 9727884 96 0.0
(read/write) 475425 475425 0 0.0
.bss 44017 44017 0 0.0
.data 1128 1128 0 0.0
.data.rel.ro 371232 371232 0 0.0
.dynamic 560 560 0 0.0
.got 55256 55256 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 492244 492196 -48 -0.0
.text 8185700 8185844 144 0.0
thermostat-no-ble arm64 (read only) 2206980 2206980 0 0.0
(read/write) 149377 149377 0 0.0
.bss 65649 65649 0 0.0
.data 1024 1024 0 0.0
.data.rel.ro 75368 75368 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 136820 136820 0 0.0
.text 1850848 1850848 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1156219 1156219 0 0.0
bss 146644 146644 0 0.0
rodata 145220 145220 0 0.0
text 789208 789208 0 0.0
p6 all-clusters-app default (read/write) 2492128 2492128 0 0.0
.bss 120080 120080 0 0.0
.data 2632 2632 0 0.0
.text 1450392 1450392 0 0.0
light-app default (read/write) 2395992 2395992 0 0.0
.bss 113544 113544 0 0.0
.data 2488 2488 0 0.0
.text 1354256 1354256 0 0.0
lock-app default (read/write) 2359536 2359536 0 0.0
.bss 113288 113288 0 0.0
.data 2448 2448 0 0.0
.text 1317800 1317800 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 893842 893842 0 0.0
bss 87432 87432 0 0.0
noinit 37160 37160 0 0.0
text 631882 631882 0 0.0

@bzbarsky-apple bzbarsky-apple mentioned this pull request Mar 14, 2022
Closed
@restyled-io restyled-io bot mentioned this pull request Mar 14, 2022
Closed
@bzbarsky-apple bzbarsky-apple force-pushed the check-fabric-on-message-fail branch from f5823fe to f04040c Compare March 14, 2022 15:02
@github-actions
Copy link

github-actions bot commented Mar 14, 2022 •
edited
Loading

PR #16157: Size comparison from d86a7d4 to f04040c

Increases (3 builds for linux)
platform target config section d86a7d4 f04040c change % change
linux chip-tool debug (read only) 10094445 10094637 192 0.0
.rodata 514965 514997 32 0.0
.text 8798229 8798389 160 0.0
chip-tool-ipv6only arm64 (read only) 9727740 9727836 96 0.0
.text 8185652 8185796 144 0.0
tv-app debug (read only) 2655553 2655889 336 0.0
.rodata 204245 204277 32 0.0
.text 2277170 2277474 304 0.0
Decreases (1 build for linux)
platform target config section d86a7d4 f04040c change % change
linux chip-tool-ipv6only arm64 .rodata 492244 492196 -48 -0.0
Full report (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section d86a7d4 f04040c change % change
cyw30739 light cyw930739m2evb_01 (read/write) 602530 602530 0 0.0
.app_xip_area 509708 509708 0 0.0
.bss 75576 75576 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) 560390 560390 0 0.0
.app_xip_area 469096 469096 0 0.0
.bss 74080 74080 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) 570490 570490 0 0.0
.app_xip_area 469548 469548 0 0.0
.bss 83384 83384 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) 918764 918764 0 0.0
(read/write) 128656 128656 0 0.0
.bss 126648 126648 0 0.0
.data 2008 2008 0 0.0
.text 918756 918756 0 0.0
BRD4161A+rpc (read only) 947568 947568 0 0.0
(read/write) 144616 144616 0 0.0
.bss 142424 142424 0 0.0
.data 2188 2188 0 0.0
.text 947560 947560 0 0.0
window-app BRD4161A (read only) 850616 850616 0 0.0
(read/write) 126632 126632 0 0.0
.bss 124768 124768 0 0.0
.data 1864 1864 0 0.0
.text 850608 850608 0 0.0
esp32 all-clusters-app c3devkit (read only) 961344 961344 0 0.0
(read/write) 1394994 1394994 0 0.0
.dram0.bss 64056 64056 0 0.0
.dram0.data 14188 14188 0 0.0
.flash.rodata 197832 197832 0 0.0
.flash.text 961344 961344 0 0.0
.iram0.text 62016 62016 0 0.0
m5stack (read only) 1016963 1016963 0 0.0
(read/write) 462204 462204 0 0.0
.dram0.bss 69576 69576 0 0.0
.dram0.data 34016 34016 0 0.0
.flash.rodata 226776 226776 0 0.0
.flash.text 1011579 1011579 0 0.0
.iram0.text 123107 123107 0 0.0
k32w light k32w061+release (read/write) 699636 699636 0 0.0
.bss 77560 77560 0 0.0
.data 1888 1888 0 0.0
.text 614388 614388 0 0.0
lock k32w061+release (read/write) 699884 699884 0 0.0
.bss 77552 77552 0 0.0
.data 1908 1908 0 0.0
.text 614624 614624 0 0.0
linux all-clusters-app debug (read only) 2465073 2465073 0 0.0
(read/write) 146384 146384 0 0.0
.bss 60640 60640 0 0.0
.data 1264 1264 0 0.0
.data.rel.ro 78728 78728 0 0.0
.dynamic 592 592 0 0.0
.got 4176 4176 0 0.0
.init 27 27 0 0.0
.init_array 936 936 0 0.0
.rodata 210757 210757 0 0.0
.text 2090994 2090994 0 0.0
bridge-app debug+rpc (read only) 1749589 1749589 0 0.0
(read/write) 92912 92912 0 0.0
.bss 47752 47752 0 0.0
.data 1952 1952 0 0.0
.data.rel.ro 38128 38128 0 0.0
.dynamic 592 592 0 0.0
.got 3928 3928 0 0.0
.init 27 27 0 0.0
.init_array 544 544 0 0.0
.rodata 143372 143372 0 0.0
.text 1490581 1490581 0 0.0
chip-tool debug (read only) 10094445 10094637 192 0.0
(read/write) 357680 357680 0 0.0
.bss 25696 25696 0 0.0
.data 1072 1072 0 0.0
.data.rel.ro 324864 324864 0 0.0
.dynamic 608 608 0 0.0
.got 4776 4776 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 514965 514997 32 0.0
.text 8798229 8798389 160 0.0
chip-tool-ipv6only arm64 (read only) 9727740 9727836 96 0.0
(read/write) 475425 475425 0 0.0
.bss 44017 44017 0 0.0
.data 1128 1128 0 0.0
.data.rel.ro 371232 371232 0 0.0
.dynamic 560 560 0 0.0
.got 55256 55256 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 492244 492196 -48 -0.0
.text 8185652 8185796 144 0.0
door-lock-app debug (read only) 1984225 1984225 0 0.0
(read/write) 119776 119776 0 0.0
.bss 50944 50944 0 0.0
.data 992 992 0 0.0
.data.rel.ro 62456 62456 0 0.0
.dynamic 592 592 0 0.0
.got 4112 4112 0 0.0
.init 27 27 0 0.0
.init_array 664 664 0 0.0
.rodata 178460 178460 0 0.0
.text 1656786 1656786 0 0.0
lighting-app debug+rpc (read only) 2115785 2115785 0 0.0
(read/write) 125160 125160 0 0.0
.bss 51776 51776 0 0.0
.data 1472 1472 0 0.0
.data.rel.ro 66408 66408 0 0.0
.dynamic 608 608 0 0.0
.got 4144 4144 0 0.0
.init 27 27 0 0.0
.init_array 736 736 0 0.0
.rodata 171004 171004 0 0.0
.text 1788786 1788786 0 0.0
ota-provider-app debug (read only) 1917489 1917489 0 0.0
(read/write) 115536 115536 0 0.0
.bss 50816 50816 0 0.0
.data 1256 1256 0 0.0
.data.rel.ro 57816 57816 0 0.0
.dynamic 608 608 0 0.0
.got 4368 4368 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 163691 163691 0 0.0
.text 1602738 1602738 0 0.0
ota-requestor-app debug (read only) 1949105 1949105 0 0.0
(read/write) 118568 118568 0 0.0
.bss 51840 51840 0 0.0
.data 1416 1416 0 0.0
.data.rel.ro 59880 59880 0 0.0
.dynamic 592 592 0 0.0
.got 4160 4160 0 0.0
.init 27 27 0 0.0
.init_array 640 640 0 0.0
.rodata 160356 160356 0 0.0
.text 1637442 1637442 0 0.0
shell debug (read only) 2410913 2410913 0 0.0
(read/write) 150184 150184 0 0.0
.bss 70600 70600 0 0.0
.data 784 784 0 0.0
.data.rel.ro 73088 73088 0 0.0
.dynamic 592 592 0 0.0
.got 4160 4160 0 0.0
.init 27 27 0 0.0
.init_array 920 920 0 0.0
.rodata 207538 207538 0 0.0
.text 2047138 2047138 0 0.0
thermostat-no-ble arm64 (read only) 2206980 2206980 0 0.0
(read/write) 149377 149377 0 0.0
.bss 65649 65649 0 0.0
.data 1024 1024 0 0.0
.data.rel.ro 75368 75368 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 136820 136820 0 0.0
.text 1850848 1850848 0 0.0
tv-app debug (read only) 2655553 2655889 336 0.0
(read/write) 250776 250776 0 0.0
.bss 167840 167840 0 0.0
.data 3104 3104 0 0.0
.data.rel.ro 73800 73800 0 0.0
.dynamic 592 592 0 0.0
.got 4544 4544 0 0.0
.init 27 27 0 0.0
.init_array 880 880 0 0.0
.rodata 204245 204277 32 0.0
.text 2277170 2277474 304 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2352748 2352748 0 0.0
.bss 186652 186652 0 0.0
.data 5752 5752 0 0.0
.text 1315348 1315348 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1156843 1156843 0 0.0
bss 146644 146644 0 0.0
rodata 145364 145364 0 0.0
text 789688 789688 0 0.0
p6 all-clusters-app default (read/write) 2492128 2492128 0 0.0
.bss 120080 120080 0 0.0
.data 2632 2632 0 0.0
.text 1450392 1450392 0 0.0
light-app default (read/write) 2395992 2395992 0 0.0
.bss 113544 113544 0 0.0
.data 2488 2488 0 0.0
.text 1354256 1354256 0 0.0
lock-app default (read/write) 2359536 2359536 0 0.0
.bss 113288 113288 0 0.0
.data 2448 2448 0 0.0
.text 1317800 1317800 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 893842 893842 0 0.0
bss 87432 87432 0 0.0
noinit 37160 37160 0 0.0
text 631882 631882 0 0.0

@bzbarsky-apple bzbarsky-apple merged commit e6a96ac into project-chip:master Mar 14, 2022
@bzbarsky-apple bzbarsky-apple deleted the check-fabric-on-message-fail branch March 14, 2022 20:52
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this pull request Apr 14, 2022
@bzbarsky-apple @andrei-menzopol
Don't try to do something with message delivery failures on other fab…
7d85c32 
…rics. (project-chip#16157)

* Don't try to do something with message delivery failures on other fabrics.

If we have DeviceController instances for multiple fabrics around, all
will be notified via OnFirstMessageDeliveryFailed on message delivery
failure for any message.  Ignore the notifications unless they are for
a message over a CASE session for the fabric the controller is
associated with (in which case it might actually make sense for that
controller to act on the notification by redoing operational address
discovery and CASE).

* Address review comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Damian-Nordic Damian-Nordic Damian-Nordic approved these changes

@andy31415 andy31415 andy31415 approved these changes

@tcarmelveilleux tcarmelveilleux tcarmelveilleux approved these changes

@anush-apple anush-apple Awaiting requested review from anush-apple

@austinh0 austinh0 Awaiting requested review from austinh0

@Byungjoo-Lee Byungjoo-Lee Awaiting requested review from Byungjoo-Lee

@carol-apple carol-apple Awaiting requested review from carol-apple

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@franck-apple franck-apple Awaiting requested review from franck-apple

@gjc13 gjc13 Awaiting requested review from gjc13

@harimau-qirex harimau-qirex Awaiting requested review from harimau-qirex

@hawk248 hawk248 Awaiting requested review from hawk248

@harsha-rajendran harsha-rajendran Awaiting requested review from harsha-rajendran

@isiu-apple isiu-apple Awaiting requested review from isiu-apple

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@kghost kghost Awaiting requested review from kghost

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@mlepage-google mlepage-google Awaiting requested review from mlepage-google

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tecimovic tecimovic Awaiting requested review from tecimovic

@vijs vijs Awaiting requested review from vijs

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@woody-apple woody-apple Awaiting requested review from woody-apple

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yufengwangca yufengwangca Awaiting requested review from yufengwangca

Assignees
No one assigned
Labels
controller review - approved transport
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bzbarsky-apple @tcarmelveilleux @andy31415 @Damian-Nordic