[dns-sd] Extended discovery improvements #16290
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pullapprove
bot
requested review from
andy31415,
anush-apple,
austinh0,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
dhrishi,
electrocucaracha,
emargolis,
erjiaqing,
franck-apple,
gjc13,
hawk248,
harsha-rajendran,
isiu-apple,
jelderton,
jepenven-silabs,
jmartinez-silabs,
kghost,
kpschoedel,
lazarkov,
LuDuda,
lzgrablic02,
msandstedt and
robszewczyk
andy31415
approved these changes
Mar 16, 2022
bzbarsky-apple
requested changes
Mar 17, 2022
chrisdecenzo
requested changes
Mar 18, 2022
Damian-Nordic
force-pushed
the
discovery
branch
from
fa00e05
to
547ade6
Compare
|
PR #16290: Size comparison from 193b4d0 to 547ade6 Increases (4 builds for cyw30739, telink)
Decreases (22 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6)
Full report (26 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
|
PR #16290: Size comparison from 193b4d0 to f1f9fba Increases (4 builds for cyw30739, telink)
Decreases (22 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6)
Full report (26 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
bzbarsky-apple
approved these changes
Mar 22, 2022
According to the spec, each time a commissionable node service is advertised and the device is not in the commissioning mode, the advertising should be treated as Extended Discovery. The current code, however, makes a distinction whether the device is on any fabric or not.
The spec recommends that the extended discovery by default time out after a period recommended in the "Announcement duration" section. Change the default from "no timeout" to 15 minutes. Use DefaultStorageKeyAllocator when persisting the timeout. Make extended discovery logs more meaningful for a non-implementer and remove some irrelevant messages.
Discriminator must be now explicitly specified in Linux apps. Previously the Cirque tests were passing only by chance.
Most platforms have it enabled, by default, and those that don't, have extended discovery disabled, too, which covers the case when the commissioning window is closed. On the other hand, when the commissioning window is open, the device should always try to advertise over DNS-SD.
Damian-Nordic
force-pushed
the
discovery
branch
from
f1f9fba
to
deeb2fd
Compare
|
PR #16290: Size comparison from 2e658fb to deeb2fd Increases (4 builds for cyw30739, telink)
Decreases (31 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6)
Full report (35 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
chrisdecenzo
approved these changes
Mar 23, 2022
andrei-menzopol
pushed a commit
to andrei-menzopol/connectedhomeip
that referenced
this pull request
Apr 14, 2022
* [dns-sd] Change extended discovery condition According to the spec, each time a commissionable node service is advertised and the device is not in the commissioning mode, the advertising should be treated as Extended Discovery. The current code, however, makes a distinction whether the device is on any fabric or not. * [dns-sd] Set default extended discovery timeout to 15 minutes The spec recommends that the extended discovery by default time out after a period recommended in the "Announcement duration" section. Change the default from "no timeout" to 15 minutes. Use DefaultStorageKeyAllocator when persisting the timeout. Make extended discovery logs more meaningful for a non-implementer and remove some irrelevant messages. * Fix Cirque tests Discriminator must be now explicitly specified in Linux apps. Previously the Cirque tests were passing only by chance. * Remove CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONABLE_DISCOVERY Most platforms have it enabled, by default, and those that don't, have extended discovery disabled, too, which covers the case when the commissioning window is closed. On the other hand, when the commissioning window is open, the device should always try to advertise over DNS-SD. * Fix build * Follow KVS key convention
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Change overview
Fixes Extended timeout configuration value not initialized ? #11124
Testing
Built a sample with extended discovery disabled and observed that commissionable node advertising doesn't start until the commissioning window is opened.