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

[shell] Add "dns browse commissioner" command #8836

Merged

Conversation

Damian-Nordic
Copy link
Contributor

@Damian-Nordic Damian-Nordic commented Aug 6, 2021

Problem

The existing "dns browse" command searches for Matter commissionable nodes only and we need some way to verify commissioner discovery for TE5.

Change overview

Split "dns browse" command into "dns browse commissionable" and "dns browse commissioner" to provide means to test commissioner discovery on all devices.
Also, allow to specify a subtype for both commands and fix buffer sizes in the OpenThread-based DNS implementation to
accomodate filtering by subtypes.

Testing

Tested manually that commands for both commissionable and commissioner discovery work correctly on nRF52840 DK (although other Thread platforms should behave the same since the code is shared) when the commissionable and commissioner node services are published by other devices in the same Thread network or in the Wi-Fi/Ethernet network that OTBR is connected to. Note that there's an issue in OpenThread with publishing services with subtypes, so the OTBR must be built with the following patch to test this change: openthread/openthread#6901.

@sharadb-amazon
Copy link
Contributor

A couple of my previous PRs did something similar:
#7193
#7900

@Damian-Nordic
Copy link
Contributor Author

A couple of my previous PRs did something similar:
#7193
#7900

Yes, I've seen that, your PRs added the commissioner discovery to the controller apps, but we also need a way to test the feature on embedded devices and the shell commands can be built into any app on any platform.

Damian-Nordic and others added 2 commits August 11, 2021 08:17
The existing "dns browse" command searches for Matter
commissionable nodes only. Split it into "dns browse
commissionable" and "dns browse commissioner" to provide
means to test commissioner discovery on all devices.

Also, allow to specify a subtype for both commands and fix
buffer sizes in the OpenThread-based DNS implementation to
accomodate filtering by subtypes.
@Damian-Nordic Damian-Nordic force-pushed the commissioner-discovery branch from 1f8b551 to f134908 Compare August 11, 2021 06:19
@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from b3f7d9f

File Section File VM
chip-shell.elf text 656 656
chip-shell.elf rodata 416 416
chip-shell.elf bss 0 260
chip-shell.elf [LOAD #3 [RW]] 0 -4
chip-lock.elf text 32 32
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_info,0,3075
.debug_loc,0,1208
.strtab,0,722
text,656,656
.debug_line,0,541
rodata,416,416
.symtab,0,352
bss,260,0
.debug_abbrev,0,168
.debug_ranges,0,128
.debug_frame,0,88
.debug_aranges,0,16
.shstrtab,0,-2
[LOAD #3 [RW]],-4,0
.debug_str,0,-288

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.debug_info,0,596
.debug_loc,0,196
.debug_line,0,61
.debug_ranges,0,32
.debug_str,0,32
text,32,32
.debug_abbrev,0,15
.debug_frame,0,4


@Damian-Nordic Damian-Nordic merged commit 6ffaf3a into project-chip:master Aug 11, 2021
@github-actions
Copy link

Size increase report for "esp32-example-build" from b3f7d9f

File Section File VM
chip-bridge-app.elf .flash.text -48 -48
chip-shell.elf .flash.text 624 624
chip-shell.elf .flash.rodata 416 416
chip-shell.elf .dram0.bss 0 264
chip-temperature-measurement-app.elf .flash.text 60 60
chip-all-clusters-app.elf .flash.text 572 572
chip-all-clusters-app.elf .flash.rodata 424 424
chip-all-clusters-app.elf .dram0.bss 0 264
chip-all-clusters-app.elf .dram0.heap_start 0 8
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-bridge-app.elf and ./pull_artifact/chip-bridge-app.elf:

sections,vmsize,filesize
[Unmapped],0,48
.flash.text,-48,-48

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
[Unmapped],0,3056
.debug_info,0,2588
.debug_line,0,953
.strtab,0,723
.debug_loc,0,658
.flash.text,624,624
.flash.rodata,416,416
.dram0.bss,264,0
.debug_abbrev,0,142
.debug_frame,0,48
.debug_ranges,0,48
[2 Others],0,-32
.xt.prop._ZTVN4chip5Shell24DnsShellResolverDelegateE,0,-52
.xt.prop._ZN4chip5Shell24DnsShellResolverDelegate24OnNodeIdResolutionFailedERKNS_6PeerIdENS_9ChipErrorE,0,-76
.xt.prop._ZN4chip5Shell24DnsShellResolverDelegateD0Ev,0,-76
.xt.prop._ZN4chip5Shell24DnsShellResolverDelegateD2Ev,0,-76
.xt.lit._ZN4chip5Shell24DnsShellResolverDelegate23OnNodeDiscoveryCompleteERKNS_4Mdns18DiscoveredNodeDataE,0,-88
.xt.prop._ZN4chip5Shell24DnsShellResolverDelegate16OnNodeIdResolvedERKNS_4Mdns16ResolvedNodeDataE,0,-124
.debug_str,0,-209
.xt.prop._ZN4chip5Shell24DnsShellResolverDelegate23OnNodeDiscoveryCompleteERKNS_4Mdns18DiscoveredNodeDataE,0,-224
.shstrtab,0,-779

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
.flash.text,60,60
[Unmapped],0,-60

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_info,0,2231
.strtab,0,655
.debug_line,0,646
.debug_loc,0,602
.flash.text,572,572
.flash.rodata,424,424
.dram0.bss,264,0
.symtab,0,144
.debug_abbrev,0,111
.debug_frame,0,76
[ELF Headers],0,40
.debug_ranges,0,32
.debug_aranges,0,16
.dram0.heap_start,8,0
.shstrtab,0,-3
.debug_str,0,-494
[Unmapped],0,-996

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize


@Damian-Nordic Damian-Nordic deleted the commissioner-discovery branch August 11, 2021 07:14
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* [shell] Add "dns browse commissioner" command

The existing "dns browse" command searches for Matter
commissionable nodes only. Split it into "dns browse
commissionable" and "dns browse commissioner" to provide
means to test commissioner discovery on all devices.

Also, allow to specify a subtype for both commands and fix
buffer sizes in the OpenThread-based DNS implementation to
accomodate filtering by subtypes.

* Restyled by clang-format

Co-authored-by: Restyled.io <[email protected]>
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.

7 participants