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

Android: Synchronizing on the use of NsdManager.resolveService() #23515

Merged
merged 2 commits into from
Nov 8, 2022
Merged

Android: Synchronizing on the use of NsdManager.resolveService() #23515

merged 2 commits into from
Nov 8, 2022

Conversation

sharadb-amazon
Copy link
Contributor

Fixes #23514

Problem

The Android NsdManager seems capable of only resolving one service at a time. If it is called to resolve a service while it already is busy with a previous request, it calls back the second caller's ResolveListener with a FAILURE_ALREADY_ACTIVE error.

This leads to issues with running discovery and resolution from any app code if Matter SDK's NsdManagerServiceResolver.java is running service resolution in parallel. For example, Commissioner discovery/resolution may fail if Operational discovery/resolution is already in progress - as manifested in the Android tv-casting-app bug cited above. Further more, this may lead to issues with any usage of NsdManager's resolveService API for any use case (Matter related or otherwise)

Change summary

This change adds a NsdManagerResolverAvailState to the Matter NsdManagerServiceResolver and allows setting it optionally via an overloaded constructor. If an app provides this NsdManagerResolverAvailState object, it can use it to synchronize with the Matter NsdManagerServiceResolver on the usage of NsdManager's resolveService API. This helps avoid failing with the FAILURE_ALREADY_ACTIVE error when using the NsdManager (in Matter SDK and/or app side code)

Testing

Tested the fix with the android tv-casting-app by following the repro steps in the issue cited above.

@sharadb-amazon sharadb-amazon added bug Something isn't working android labels Nov 6, 2022
@github-actions
Copy link

github-actions bot commented Nov 6, 2022 •
edited
Loading

PR #23515: Size comparison from 1ee863e to 181a9e4

Increases (3 builds for bl702, qpg, telink)
platform target config section 1ee863e 181a9e4 change % change
bl702 lighting-app bl702 .debug_info 38074142 38074143 1 0.0
.text 957110 957112 2 0.0
qpg lighting-app qpg6105+debug (read/write) 1149552 1149560 8 0.0
.text 596652 596660 8 0.0
telink all-clusters-app tlsr9518adk80d text 648602 648604 2 0.0
Decreases (4 builds for bl702, k32w, nrfconnect, telink)
platform target config section 1ee863e 181a9e4 change % change
bl702 lighting-app bl702+rpc (read/write) 1284123 1284107 -16 -0.0
.debug_info 4199138 41991385 -1 -0.0
.text 1030818 1030816 -2 -0.0
k32w light k32w0+release (read/write) 671288 671272 -16 -0.0
.text 591676 591660 -16 -0.0
nrfconnect all-clusters-minimal-app nrf52840dk_nrf52840 text 805446 805442 -4 -0.0
telink lighting-app tlsr9518adk80d text 624080 624078 -2 -0.0
Full report (17 builds for bl702, k32w, linux, mbed, nrfconnect, qpg, telink)
platform target config section 1ee863e 181a9e4 change % change
bl702 lighting-app bl702 0 0 0 0.0
(read only) 3262 3262 0 0.0
(read/write) 1188179 1188179 0 0.0
.bleromro 6296 6296 0 0.0
.bleromrw 124 124 0 0.0
.boot2 688 688 0 0.0
.bss 67126 67126 0 0.0
.bss_psram 29696 29696 0 0.0
.comment 48 48 0 0.0
.data 4280 4280 0 0.0
.debug_abbrev 1514432 1514432 0 0.0
.debug_aranges 132032 132032 0 0.0
.debug_frame 484600 484600 0 0.0
.debug_info 38074142 38074143 1 0.0
.debug_line 5271016 5271016 0 0.0
.debug_loc 3373259 3373259 0 0.0
.debug_ranges 359736 359736 0 0.0
.debug_str 3447472 3447472 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 116152 116152 0 0.0
.rsvd 3188 3188 0 0.0
.shstrtab 293 293 0 0.0
.stack 2048 2048 0 0.0
.strtab 565752 565752 0 0.0
.symtab 171984 171984 0 0.0
.tcm_data 36 36 0 0.0
.tcmcode 3262 3262 0 0.0
.text 957110 957112 2 0.0
bl702+rpc 0 0 0 0.0
(read only) 3262 3262 0 0.0
(read/write) 1284123 1284107 -16 -0.0
.bleromro 6296 6296 0 0.0
.bleromrw 124 124 0 0.0
.boot2 688 688 0 0.0
.bss 75158 75158 0 0.0
.bss_psram 29936 29936 0 0.0
.comment 48 48 0 0.0
.data 4816 4816 0 0.0
.debug_abbrev 1651848 1651848 0 0.0
.debug_aranges 139536 139536 0 0.0
.debug_frame 509972 509972 0 0.0
.debug_info 4199138 41991385 -1 -0.0
.debug_line 5647163 5647163 0 0.0
.debug_loc 3566043 3566043 0 0.0
.debug_ranges 382536 382536 0 0.0
.debug_str 3843622 3843622 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 129560 129560 0 0.0
.rsvd 3188 3188 0 0.0
.shstrtab 293 293 0 0.0
.stack 2048 2048 0 0.0
.strtab 624925 624925 0 0.0
.symtab 189792 189792 0 0.0
.tcm_data 36 36 0 0.0
.tcmcode 3262 3262 0 0.0
.text 1030818 1030816 -2 -0.0
k32w contact k32w0+release (read/write) 661244 661244 0 0.0
.bss 77024 77024 0 0.0
.data 2104 2104 0 0.0
.text 563004 563004 0 0.0
light k32w0+release (read/write) 671288 671272 -16 -0.0
.bss 74824 74824 0 0.0
.data 2060 2060 0 0.0
.text 591676 591660 -16 -0.0
lock k32w0+release (read/write) 632764 632764 0 0.0
.bss 75584 75584 0 0.0
.data 2080 2080 0 0.0
.text 552372 552372 0 0.0
linux chip-tool-ipv6only arm64 (read only) 10433484 10433484 0 0.0
(read/write) 706337 706337 0 0.0
.bss 33905 33905 0 0.0
.data 2768 2768 0 0.0
.data.rel.ro 650672 650672 0 0.0
.dynamic 560 560 0 0.0
.got 13912 13912 0 0.0
.init 24 24 0 0.0
.init_array 208 208 0 0.0
.rodata 518220 518220 0 0.0
.text 8259988 8259988 0 0.0
thermostat-no-ble arm64 (read only) 2392092 2392092 0 0.0
(read/write) 143777 143777 0 0.0
.bss 55393 55393 0 0.0
.data 1816 1816 0 0.0
.data.rel.ro 77328 77328 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 144724 144724 0 0.0
.text 2004000 2004000 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2452192 2452192 0 0.0
.bss 215012 215012 0 0.0
.data 5872 5872 0 0.0
.text 1414836 1414836 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1187759 1187759 0 0.0
bss 145285 145285 0 0.0
rodata 144436 144436 0 0.0
text 816956 816956 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1167107 1167107 0 0.0
bss 144512 144512 0 0.0
rodata 136224 136224 0 0.0
text 805446 805442 -4 -0.0
qpg lighting-app qpg6105+debug (read/write) 1149552 1149560 8 0.0
.bss 110596 110596 0 0.0
.data 848 848 0 0.0
.text 596652 596660 8 0.0
lock-app qpg6105+debug (read/write) 1114352 1114352 0 0.0
.bss 106372 106372 0 0.0
.data 836 836 0 0.0
.text 561448 561448 0 0.0
telink all-clusters-app tlsr9518adk80d (read/write) 936764 936764 0 0.0
bss 81616 81616 0 0.0
noinit 43440 43440 0 0.0
text 648602 648604 2 0.0
all-clusters-minimal-app tlsr9518adk80d (read/write) 874740 874740 0 0.0
bss 80840 80840 0 0.0
noinit 43440 43440 0 0.0
text 612572 612572 0 0.0
light-switch-app tlsr9518adk80d (read/write) 790236 790236 0 0.0
bss 72464 72464 0 0.0
noinit 43520 43520 0 0.0
text 554100 554100 0 0.0
lighting-app tlsr9518adk80d (read/write) 874460 874460 0 0.0
bss 82232 82232 0 0.0
noinit 43520 43520 0 0.0
text 624080 624078 -2 -0.0
ota-requestor-app tlsr9518adk80d (read/write) 804728 804728 0 0.0
bss 74036 74036 0 0.0
noinit 43520 43520 0 0.0
text 565480 565480 0 0.0

@github-actions
Copy link

github-actions bot commented Nov 7, 2022

PR #23515: Size comparison from 1ee863e to 04ce083

Full report (5 builds for bl702, linux, mbed)
platform target config section 1ee863e 04ce083 change % change
bl702 lighting-app bl702 0 0 0 0.0
(read only) 3262 3262 0 0.0
(read/write) 1188179 1188179 0 0.0
.bleromro 6296 6296 0 0.0
.bleromrw 124 124 0 0.0
.boot2 688 688 0 0.0
.bss 67126 67126 0 0.0
.bss_psram 29696 29696 0 0.0
.comment 48 48 0 0.0
.data 4280 4280 0 0.0
.debug_abbrev 1514432 1514432 0 0.0
.debug_aranges 132032 132032 0 0.0
.debug_frame 484600 484600 0 0.0
.debug_info 38074142 38074142 0 0.0
.debug_line 5271016 5271016 0 0.0
.debug_loc 3373259 3373259 0 0.0
.debug_ranges 359736 359736 0 0.0
.debug_str 3447472 3447472 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 116152 116152 0 0.0
.rsvd 3188 3188 0 0.0
.shstrtab 293 293 0 0.0
.stack 2048 2048 0 0.0
.strtab 565752 565752 0 0.0
.symtab 171984 171984 0 0.0
.tcm_data 36 36 0 0.0
.tcmcode 3262 3262 0 0.0
.text 957110 957110 0 0.0
bl702+rpc 0 0 0 0.0
(read only) 3262 3262 0 0.0
(read/write) 1284123 1284123 0 0.0
.bleromro 6296 6296 0 0.0
.bleromrw 124 124 0 0.0
.boot2 688 688 0 0.0
.bss 75158 75158 0 0.0
.bss_psram 29936 29936 0 0.0
.comment 48 48 0 0.0
.data 4816 4816 0 0.0
.debug_abbrev 1651848 1651848 0 0.0
.debug_aranges 139536 139536 0 0.0
.debug_frame 509972 509972 0 0.0
.debug_info 4199138 4199138 0 0.0
.debug_line 5647163 5647163 0 0.0
.debug_loc 3566043 3566043 0 0.0
.debug_ranges 382536 382536 0 0.0
.debug_str 3843622 3843622 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 129560 129560 0 0.0
.rsvd 3188 3188 0 0.0
.shstrtab 293 293 0 0.0
.stack 2048 2048 0 0.0
.strtab 624925 624925 0 0.0
.symtab 189792 189792 0 0.0
.tcm_data 36 36 0 0.0
.tcmcode 3262 3262 0 0.0
.text 1030818 1030818 0 0.0
linux chip-tool-ipv6only arm64 (read only) 10433484 10433484 0 0.0
(read/write) 706337 706337 0 0.0
.bss 33905 33905 0 0.0
.data 2768 2768 0 0.0
.data.rel.ro 650672 650672 0 0.0
.dynamic 560 560 0 0.0
.got 13912 13912 0 0.0
.init 24 24 0 0.0
.init_array 208 208 0 0.0
.rodata 518220 518220 0 0.0
.text 8259988 8259988 0 0.0
thermostat-no-ble arm64 (read only) 2392092 2392092 0 0.0
(read/write) 143777 143777 0 0.0
.bss 55393 55393 0 0.0
.data 1816 1816 0 0.0
.data.rel.ro 77328 77328 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 144724 144724 0 0.0
.text 2004000 2004000 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2452192 2452192 0 0.0
.bss 215012 215012 0 0.0
.data 5872 5872 0 0.0
.text 1414836 1414836 0 0.0

@chrisdecenzo chrisdecenzo merged commit 439fad5 into project-chip:master Nov 8, 2022
sharadb-amazon added a commit to sharadb-amazon/connectedhomeip that referenced this pull request Nov 10, 2022
@sharadb-amazon
Android: Synchronizing on the use of NsdManager.resolveService() (pro…
8b177cd 
…ject-chip#23515)

* Android: Synchronizing on the use of NsdManager.resolveService()

* Incorporating comments from andy31415@
sharadb-amazon added a commit to sharadb-amazon/connectedhomeip that referenced this pull request Nov 11, 2022
@sharadb-amazon
Android: Synchronizing on the use of NsdManager.resolveService() (pro…
097043d 
…ject-chip#23515)

* Android: Synchronizing on the use of NsdManager.resolveService()

* Incorporating comments from andy31415@
adbridge pushed a commit to ARM-software/connectedhomeip that referenced this pull request Nov 18, 2022
@sharadb-amazon @adbridge
Android: Synchronizing on the use of NsdManager.resolveService() (pro…
ca96b12 
…ject-chip#23515)

* Android: Synchronizing on the use of NsdManager.resolveService()

* Incorporating comments from andy31415@
adbridge pushed a commit to ARM-software/connectedhomeip that referenced this pull request Nov 18, 2022
@sharadb-amazon @adbridge
Android: Synchronizing on the use of NsdManager.resolveService() (pro…
8c51d6b 
…ject-chip#23515)

* Android: Synchronizing on the use of NsdManager.resolveService()

* Incorporating comments from andy31415@
@zhenqinrui
Copy link

Excuse? I have a question, wheath it should new NsdManagerResolverAvailState? I havent' found the new NsdManagerResolverAvailState, how can i do ?

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

@chrisdecenzo chrisdecenzo chrisdecenzo approved these changes

@andy31415 andy31415 andy31415 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

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

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

@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

@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

@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

@mspang mspang Awaiting requested review from mspang

@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

@turon turon Awaiting requested review from turon

@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

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

Assignees
No one assigned
Labels
android bug Something isn't working platform review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Android: NsdManager resolver calls back with FAILURE_ALREADY_ACTIVE
4 participants
@sharadb-amazon @zhenqinrui @andy31415 @chrisdecenzo