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

Ensure ResolverProxy maintains callbacks until init is called. Ensure… #16230

Merged
merged 1 commit into from
Mar 15, 2022

Conversation

andy31415
Copy link
Contributor

… server app always initializes the global resolver

Problem

OTA on thread does not work (address resolution fails):

  • need to guarantee that address resolver is initialized
  • ResolverProxy will only store callbacks after it is initialized, but initialization is async and happens late (e.g. on SRP init, which is only available very late, after thread is set up and working).

Change overview

  • Make ResolverProxy store callbacks until init is called
  • Ensure server app initializes the global resolver.

Testing

Manual OTA announcement on a EFR device.

… server app always initializes the global resolver
@github-actions
Copy link

github-actions bot commented Mar 15, 2022

PR #16230: Size comparison from 494cfb2 to 876341d

Increases (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 494cfb2 876341d change % change
cyw30739 light cyw930739m2evb_01 (read/write) 602578 602866 288 0.0
.app_xip_area 509756 510020 264 0.1
.bss 75576 75600 24 0.0
lock cyw930739m2evb_01 (read/write) 560430 560646 216 0.0
.app_xip_area 469136 469328 192 0.0
.bss 74080 74104 24 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 570498 570530 32 0.0
.app_xip_area 469556 469564 8 0.0
.bss 83384 83408 24 0.0
efr32 lighting-app BRD4161A (read only) 919652 920004 352 0.0
(read/write) 128672 128688 16 0.0
.bss 126664 126680 16 0.0
.text 919644 919996 352 0.0
BRD4161A+rpc (read only) 948440 948808 368 0.0
(read/write) 144632 144648 16 0.0
.bss 142440 142456 16 0.0
.text 948432 948800 368 0.0
window-app BRD4161A (read only) 850392 850768 376 0.0
(read/write) 126632 126648 16 0.0
.bss 124768 124784 16 0.0
.text 850384 850760 376 0.0
esp32 all-clusters-app c3devkit (read only) 961388 961534 146 0.0
(read/write) 1394938 1395026 88 0.0
.dram0.bss 64056 64064 8 0.0
.flash.rodata 197776 197864 88 0.0
.flash.text 961388 961534 146 0.0
m5stack (read only) 1017011 1017055 44 0.0
(read/write) 462148 462244 96 0.0
.dram0.bss 69576 69592 16 0.0
.flash.rodata 226720 226800 80 0.0
.flash.text 1011627 1011671 44 0.0
k32w light k32w061+release (read/write) 699340 699568 228 0.0
.bss 77560 77576 16 0.0
.text 614092 614304 212 0.0
lock k32w061+release (read/write) 699644 699888 244 0.0
.bss 77552 77568 16 0.0
.text 614384 614612 228 0.0
linux chip-tool-ipv6only arm64 (read only) 9728308 9728564 256 0.0
.rodata 492252 492332 80 0.0
.text 8186180 8186356 176 0.0
thermostat-no-ble arm64 (read only) 2207324 2207628 304 0.0
(read/write) 149377 149409 32 0.0
.bss 65649 65681 32 0.0
.rodata 136828 136908 80 0.1
.text 1851184 1851408 224 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read/write) 2352756 2352964 208 0.0
.bss 186652 186660 8 0.0
.text 1315356 1315564 208 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1154011 1154191 180 0.0
bss 146644 146660 16 0.0
rodata 145276 145360 84 0.1
text 786976 787080 104 0.0
p6 all-clusters-app default (read/write) 2492152 2492256 104 0.0
.bss 120080 120088 8 0.0
.text 1450416 1450520 104 0.0
light-app default (read/write) 2396000 2396104 104 0.0
.bss 113544 113552 8 0.0
.text 1354264 1354368 104 0.0
lock-app default (read/write) 2359544 2359664 120 0.0
.bss 113288 113296 8 0.0
.text 1317808 1317928 120 0.0
telink lighting-app tlsr9518adk80d (read/write) 893770 894106 336 0.0
bss 87432 87448 16 0.0
text 631868 632106 238 0.0
Full report (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 494cfb2 876341d change % change
cyw30739 light cyw930739m2evb_01 (read/write) 602578 602866 288 0.0
.app_xip_area 509756 510020 264 0.1
.bss 75576 75600 24 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) 560430 560646 216 0.0
.app_xip_area 469136 469328 192 0.0
.bss 74080 74104 24 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) 570498 570530 32 0.0
.app_xip_area 469556 469564 8 0.0
.bss 83384 83408 24 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) 919652 920004 352 0.0
(read/write) 128672 128688 16 0.0
.bss 126664 126680 16 0.0
.data 2008 2008 0 0.0
.text 919644 919996 352 0.0
BRD4161A+rpc (read only) 948440 948808 368 0.0
(read/write) 144632 144648 16 0.0
.bss 142440 142456 16 0.0
.data 2188 2188 0 0.0
.text 948432 948800 368 0.0
window-app BRD4161A (read only) 850392 850768 376 0.0
(read/write) 126632 126648 16 0.0
.bss 124768 124784 16 0.0
.data 1864 1864 0 0.0
.text 850384 850760 376 0.0
esp32 all-clusters-app c3devkit (read only) 961388 961534 146 0.0
(read/write) 1394938 1395026 88 0.0
.dram0.bss 64056 64064 8 0.0
.dram0.data 14188 14188 0 0.0
.flash.rodata 197776 197864 88 0.0
.flash.text 961388 961534 146 0.0
.iram0.text 62016 62016 0 0.0
m5stack (read only) 1017011 1017055 44 0.0
(read/write) 462148 462244 96 0.0
.dram0.bss 69576 69592 16 0.0
.dram0.data 34016 34016 0 0.0
.flash.rodata 226720 226800 80 0.0
.flash.text 1011627 1011671 44 0.0
.iram0.text 123107 123107 0 0.0
k32w light k32w061+release (read/write) 699340 699568 228 0.0
.bss 77560 77576 16 0.0
.data 1888 1888 0 0.0
.text 614092 614304 212 0.0
lock k32w061+release (read/write) 699644 699888 244 0.0
.bss 77552 77568 16 0.0
.data 1908 1908 0 0.0
.text 614384 614612 228 0.0
linux chip-tool-ipv6only arm64 (read only) 9728308 9728564 256 0.0
(read/write) 475441 475441 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 55264 55264 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 492252 492332 80 0.0
.text 8186180 8186356 176 0.0
thermostat-no-ble arm64 (read only) 2207324 2207628 304 0.0
(read/write) 149377 149409 32 0.0
.bss 65649 65681 32 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 136828 136908 80 0.1
.text 1851184 1851408 224 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2352756 2352964 208 0.0
.bss 186652 186660 8 0.0
.data 5752 5752 0 0.0
.text 1315356 1315564 208 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1154011 1154191 180 0.0
bss 146644 146660 16 0.0
rodata 145276 145360 84 0.1
text 786976 787080 104 0.0
p6 all-clusters-app default (read/write) 2492152 2492256 104 0.0
.bss 120080 120088 8 0.0
.data 2632 2632 0 0.0
.text 1450416 1450520 104 0.0
light-app default (read/write) 2396000 2396104 104 0.0
.bss 113544 113552 8 0.0
.data 2488 2488 0 0.0
.text 1354264 1354368 104 0.0
lock-app default (read/write) 2359544 2359664 120 0.0
.bss 113288 113296 8 0.0
.data 2448 2448 0 0.0
.text 1317808 1317928 120 0.0
telink lighting-app tlsr9518adk80d (read/write) 893770 894106 336 0.0
bss 87432 87448 16 0.0
noinit 37160 37160 0 0.0
text 631868 632106 238 0.0

@@ -197,6 +197,8 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint
err = chip::app::InteractionModelEngine::GetInstance()->Init(&mExchangeMgr);
SuccessOrExit(err);

chip::Dnssd::Resolver::Instance().Init(DeviceLayer::UDPEndPointManager());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'd think Server.cpp is the right home for this bit of logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe as a followup? I believe I had added that bit to make CI pass, however it seems obvious now that this needs to be more generic (i.e. all servers have it) instead of just for the requestor app.

@andy31415 andy31415 merged commit b4fd95f into project-chip:master Mar 15, 2022
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this pull request Apr 14, 2022
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