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

[Darwin] Fix OTAProviderDelegate init after all controllers has been turned off and a new controller is started #21412

Merged
merged 2 commits into from
Jul 29, 2022
Merged

[Darwin] Fix OTAProviderDelegate init after all controllers has been turned off and a new controller is started #21412

merged 2 commits into from
Jul 29, 2022

Conversation

vivien-apple
Copy link
Contributor

Problem

Fix #21306

Change overview

  • Update MTROTAProviderDelegateBridge::Init so it returns an error if init fails
  • Shuffle the code so MTROTAProviderDelegateBridge::Init happens when the system state has been initialised with proper values
  • Add a test to make sure a new controller can be created after a proper cleanup of the system state

Testing

xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -only-testing:MatterTests/MTRControllerTests/testControllerWithOTAProviderDelegate

@vivien-apple vivien-apple self-assigned this Jul 29, 2022
bzbarsky-apple
bzbarsky-apple approved these changes Jul 29, 2022
View reviewed changes
src/darwin/Framework/CHIP/MTRControllerFactory.mm Outdated Show resolved Hide resolved
src/darwin/Framework/CHIP/MTRControllerFactory.mm Outdated Show resolved Hide resolved
src/darwin/Framework/CHIP/MTRControllerFactory.mm Outdated Show resolved Hide resolved
src/darwin/Framework/CHIPTests/MTRControllerTests.m Outdated Show resolved Hide resolved
src/darwin/Framework/CHIPTests/MTRControllerTests.m Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Jul 29, 2022 •
edited
Loading

PR #21412: Size comparison from 4c52019 to da06984

Increases (1 build for nrfconnect)
platform target config section 4c52019 da06984 change % change
nrfconnect all-clusters-minimal-app nrf52840dk_nrf52840 text 798332 798336 4 0.0
Decreases (2 builds for telink)
platform target config section 4c52019 da06984 change % change
telink light-switch-app tlsr9518adk80d text 568540 568538 -2 -0.0
lighting-app tlsr9518adk80d (read/write) 824920 824912 -8 -0.0
text 585430 585428 -2 -0.0
Full report (14 builds for bl602, cyw30739, k32w, linux, mbed, nrfconnect, telink)
platform target config section 4c52019 da06984 change % change
bl602 lighting-app bl602 (read/write) 1381618 1381618 0 0.0
.bss 117618 117618 0 0.0
.data 4480 4480 0 0.0
.text 1051888 1051888 0 0.0
bl602+rpc (read/write) 1427034 1427034 0 0.0
.bss 125058 125058 0 0.0
.data 4600 4600 0 0.0
.text 1083552 1083552 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 583414 583414 0 0.0
.app_xip_area 460232 460232 0 0.0
.bss 65624 65624 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) 589158 589158 0 0.0
.app_xip_area 461248 461248 0 0.0
.bss 70352 70352 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) 588970 588970 0 0.0
.app_xip_area 466604 466604 0 0.0
.bss 64864 64864 0 0.0
.data 688 688 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
k32w light k32w0+release (read/write) 642800 642800 0 0.0
.bss 69712 69712 0 0.0
.data 2044 2044 0 0.0
.text 568316 568316 0 0.0
lock k32w0+release (read/write) 699920 699920 0 0.0
.bss 70152 70152 0 0.0
.data 2052 2052 0 0.0
.text 624988 624988 0 0.0
linux chip-tool-ipv6only arm64 (read only) 9878692 9878692 0 0.0
(read/write) 689041 689041 0 0.0
.bss 32897 32897 0 0.0
.data 3272 3272 0 0.0
.data.rel.ro 634336 634336 0 0.0
.dynamic 560 560 0 0.0
.got 13584 13584 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 468132 468132 0 0.0
.text 7795428 7795428 0 0.0
thermostat-no-ble arm64 (read only) 2343516 2343516 0 0.0
(read/write) 141633 141633 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 4992 4992 0 0.0
.init 24 24 0 0.0
.init_array 408 408 0 0.0
.rodata 138964 138964 0 0.0
.text 1967312 1967312 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2450672 2450672 0 0.0
.bss 214516 214516 0 0.0
.data 5872 5872 0 0.0
.text 1413316 1413316 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1172931 1172931 0 0.0
bss 143120 143120 0 0.0
rodata 141828 141828 0 0.0
text 809028 809028 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1152967 1152967 0 0.0
bss 142356 142356 0 0.0
rodata 133360 133360 0 0.0
text 798332 798336 4 0.0
telink light-switch-app tlsr9518adk80d (read/write) 804472 804472 0 0.0
bss 70964 70964 0 0.0
noinit 43488 43488 0 0.0
text 568540 568538 -2 -0.0
lighting-app tlsr9518adk80d (read/write) 824920 824912 -8 -0.0
bss 71808 71808 0 0.0
noinit 43488 43488 0 0.0
text 585430 585428 -2 -0.0

@vivien-apple
Add a test named testControllerWithOTAProviderDelegate to ensure that…
f6b7f35 
… starting a new controller once all the controllers has been shut down does not fail
@vivien-apple vivien-apple force-pushed the Darwin_OTAProviderDelegate_StartupFix branch from da06984 to 9dca574 Compare July 29, 2022 14:38
@vivien-apple vivien-apple force-pushed the Darwin_OTAProviderDelegate_StartupFix branch from 9dca574 to 91ec208 Compare July 29, 2022 14:46
@bzbarsky-apple bzbarsky-apple enabled auto-merge (squash) July 29, 2022 14:49
@vivien-apple vivien-apple force-pushed the Darwin_OTAProviderDelegate_StartupFix branch from 91ec208 to ad89379 Compare July 29, 2022 14:51
@vivien-apple
Copy link
Contributor Author

Argh. I forced push an unrelated typo fix on this branch!
I will git reflog the right one...

@vivien-apple vivien-apple force-pushed the Darwin_OTAProviderDelegate_StartupFix branch from ad89379 to 91ec208 Compare July 29, 2022 14:54
@github-actions
Copy link

github-actions bot commented Jul 29, 2022 •
edited
Loading

PR #21412: Size comparison from 19cc9e7 to 91ec208

Increases (1 build for cc13x2_26x2)
platform target config section 19cc9e77 91ec208 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 668603 668611 8 0.0
.text 580092 580100 8 0.0
Decreases (3 builds for bl602, cc13x2_26x2, telink)
platform target config section 19cc9e77 91ec208 change % change
bl602 lighting-app bl602 (read/write) 1381770 1381762 -8 -0.0
.text 1052040 1052036 -4 -0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read/write) 182740 182732 -8 -0.0
telink light-switch-app tlsr9518adk80d (read/write) 804708 804700 -8 -0.0
text 568710 568708 -2 -0.0
Full report (32 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 19cc9e77 91ec208 change % change
bl602 lighting-app bl602 (read/write) 1381770 1381762 -8 -0.0
.bss 117618 117618 0 0.0
.data 4480 4480 0 0.0
.text 1052040 1052036 -4 -0.0
bl602+rpc (read/write) 1427186 1427186 0 0.0
.bss 125058 125058 0 0.0
.data 4600 4600 0 0.0
.text 1083704 1083704 0 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 668603 668611 8 0.0
(read/write) 182740 182732 -8 -0.0
.bss 74236 74236 0 0.0
.data 3372 3372 0 0.0
.rodata 88195 88195 0 0.0
.text 580092 580100 8 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 634195 634195 0 0.0
(read/write) 157820 157820 0 0.0
.bss 73532 73532 0 0.0
.data 3372 3372 0 0.0
.rodata 77411 77411 0 0.0
.text 556460 556460 0 0.0
lock-ftd LP_CC2652R7 (read only) 671583 671583 0 0.0
(read/write) 169952 169952 0 0.0
.bss 71316 71316 0 0.0
.data 3296 3296 0 0.0
.rodata 76263 76263 0 0.0
.text 594840 594840 0 0.0
lock-mtd LP_CC2652R7 (read only) 653683 653683 0 0.0
(read/write) 183540 183540 0 0.0
.bss 67004 67004 0 0.0
.data 3296 3296 0 0.0
.rodata 100875 100875 0 0.0
.text 552328 552328 0 0.0
pump-app LP_CC2652R7 (read only) 681167 681167 0 0.0
(read/write) 161200 161200 0 0.0
.bss 71380 71380 0 0.0
.data 3296 3296 0 0.0
.rodata 88919 88919 0 0.0
.text 591764 591764 0 0.0
pump-controller-app LP_CC2652R7 (read only) 666743 666743 0 0.0
(read/write) 175760 175760 0 0.0
.bss 71516 71516 0 0.0
.data 3292 3292 0 0.0
.rodata 84743 84743 0 0.0
.text 581520 581520 0 0.0
shell LP_CC2652R7 (read only) 661302 661302 0 0.0
(read/write) 185560 185560 0 0.0
.bss 76556 76556 0 0.0
.data 3376 3376 0 0.0
.rodata 85166 85166 0 0.0
.text 575820 575820 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 583502 583502 0 0.0
.app_xip_area 460320 460320 0 0.0
.bss 65624 65624 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) 589254 589254 0 0.0
.app_xip_area 461344 461344 0 0.0
.bss 70352 70352 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) 589194 589194 0 0.0
.app_xip_area 466828 466828 0 0.0
.bss 64864 64864 0 0.0
.data 688 688 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read/write) 1088248 1088248 0 0.0
.bss 133260 133260 0 0.0
.data 2064 2064 0 0.0
.text 952904 952904 0 0.0
BRD4161A+rpc (read/write) 1142556 1142556 0 0.0
.bss 149940 149940 0 0.0
.data 2276 2276 0 0.0
.text 990320 990320 0 0.0
BRD4161A+rs911x (read/write) 974136 974136 0 0.0
.bss 161736 161736 0 0.0
.data 2048 2048 0 0.0
.text 810332 810332 0 0.0
lock-app BRD4161A+wf200 (read/write) 1128616 1128616 0 0.0
.bss 144368 144368 0 0.0
.data 2056 2056 0 0.0
.text 982172 982172 0 0.0
window-app BRD4161A (read/write) 1081540 1081540 0 0.0
.bss 134732 134732 0 0.0
.data 2092 2092 0 0.0
.text 944692 944692 0 0.0
esp32 all-clusters-app c3devkit (read only) 1022964 1022964 0 0.0
(read/write) 1486338 1486338 0 0.0
.dram0.bss 70296 70296 0 0.0
.dram0.data 14600 14600 0 0.0
.flash.rodata 216000 216000 0 0.0
.flash.text 1022964 1022964 0 0.0
.iram0.text 62902 62902 0 0.0
m5stack (read only) 1076451 1076451 0 0.0
(read/write) 488376 488376 0 0.0
.dram0.bss 75808 75808 0 0.0
.dram0.data 34144 34144 0 0.0
.flash.rodata 246428 246428 0 0.0
.flash.text 1071067 1071067 0 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w0+release (read/write) 642896 642896 0 0.0
.bss 69712 69712 0 0.0
.data 2044 2044 0 0.0
.text 568412 568412 0 0.0
lock k32w0+release (read/write) 700016 700016 0 0.0
.bss 70152 70152 0 0.0
.data 2052 2052 0 0.0
.text 625084 625084 0 0.0
linux chip-tool-ipv6only arm64 (read only) 9879540 9879540 0 0.0
(read/write) 689073 689073 0 0.0
.bss 32897 32897 0 0.0
.data 3272 3272 0 0.0
.data.rel.ro 634360 634360 0 0.0
.dynamic 560 560 0 0.0
.got 13584 13584 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 468196 468196 0 0.0
.text 7796180 7796180 0 0.0
thermostat-no-ble arm64 (read only) 2344316 2344316 0 0.0
(read/write) 141649 141649 0 0.0
.bss 55313 55313 0 0.0
.data 1672 1672 0 0.0
.data.rel.ro 75904 75904 0 0.0
.dynamic 560 560 0 0.0
.got 4992 4992 0 0.0
.init 24 24 0 0.0
.init_array 408 408 0 0.0
.rodata 139028 139028 0 0.0
.text 1968016 1968016 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2450880 2450880 0 0.0
.bss 214516 214516 0 0.0
.data 5872 5872 0 0.0
.text 1413524 1413524 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1173135 1173135 0 0.0
bss 143120 143120 0 0.0
rodata 141888 141888 0 0.0
text 809184 809184 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1153187 1153187 0 0.0
bss 142356 142356 0 0.0
rodata 133420 133420 0 0.0
text 798496 798496 0 0.0
p6 all-clusters-app default (read only) 881560 881560 0 0.0
(read/write) 1687196 1687196 0 0.0
.bss 149136 149136 0 0.0
.data 2648 2648 0 0.0
.text 1527024 1527024 0 0.0
all-clusters-minimal-app default (read only) 882280 882280 0 0.0
(read/write) 1631308 1631308 0 0.0
.bss 148416 148416 0 0.0
.data 2648 2648 0 0.0
.text 1471856 1471856 0 0.0
light-app default (read only) 890584 890584 0 0.0
(read/write) 1552716 1552716 0 0.0
.bss 140320 140320 0 0.0
.data 2440 2440 0 0.0
.text 1401568 1401568 0 0.0
lock-app default (read only) 886112 886112 0 0.0
(read/write) 1590132 1590132 0 0.0
.bss 144776 144776 0 0.0
.data 2456 2456 0 0.0
.text 1434512 1434512 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 804708 804700 -8 -0.0
bss 70964 70964 0 0.0
noinit 43488 43488 0 0.0
text 568710 568708 -2 -0.0
lighting-app tlsr9518adk80d (read/write) 825140 825140 0 0.0
bss 71808 71808 0 0.0
noinit 43488 43488 0 0.0
text 585596 585596 0 0.0

@bzbarsky-apple bzbarsky-apple merged commit f49327a into project-chip:master Jul 29, 2022
github-actions bot pushed a commit that referenced this pull request Jul 29, 2022
@vivien-apple @web-flow
[Darwin] Fix OTAProviderDelegate init after all controllers has been …
abd196a 
…turned off and a new controller is started (#21412)

* Add a test named testControllerWithOTAProviderDelegate to ensure that starting a new controller once all the controllers has been shut down does not fail

* Fix OTA delegate bridge init on Darwin
woody-apple added a commit that referenced this pull request Jul 29, 2022
@woody-apple @vivien-apple
[Darwin] Fix OTAProviderDelegate init after all controllers has been …
bd8943b 
…turned off and a new controller is started (#21412) (#21426)

* Add a test named testControllerWithOTAProviderDelegate to ensure that starting a new controller once all the controllers has been shut down does not fail

* Fix OTA delegate bridge init on Darwin

Co-authored-by: Vivien Nicolas <[email protected]>
isiu-apple pushed a commit to isiu-apple/connectedhomeip that referenced this pull request Sep 16, 2022
@vivien-apple @isiu-apple
[Darwin] Fix OTAProviderDelegate init after all controllers has been …
e673ef5 
…turned off and a new controller is started (project-chip#21412)

* Add a test named testControllerWithOTAProviderDelegate to ensure that starting a new controller once all the controllers has been shut down does not fail

* Fix OTA delegate bridge init on Darwin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bzbarsky-apple bzbarsky-apple bzbarsky-apple approved these changes

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

@arkq arkq Awaiting requested review from arkq

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

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

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@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

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

@hawk248 hawk248 Awaiting requested review from hawk248

@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

@jtung-apple jtung-apple Awaiting requested review from jtung-apple

@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

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@rgoliver rgoliver Awaiting requested review from rgoliver

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@tecimovic tecimovic Awaiting requested review from tecimovic

@vijs vijs Awaiting requested review from vijs

@wbschiller wbschiller Awaiting requested review from wbschiller

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

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

Assignees

@vivien-apple vivien-apple

Labels
darwin fast track review - pending
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix OTA delegate bridge init on Darwin
2 participants
@vivien-apple @bzbarsky-apple