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

Platform impl for commissionable node discovery #7633

Merged

Conversation

cecille
Copy link
Contributor

@cecille cecille commented Jun 15, 2021

Problem

Currently there is no platform implementation for commissionable node discovery.
Fixes #6693

Change overview

Add platform implementation for commissionable node discovery

Testing

Tested with M5 + chip-device-ctrl compiled with chip_mdns platform.

@todo
Copy link

todo bot commented Jun 15, 2021

(cecille): These need to be changed to remove leading zeros

// TODO(cecille): These need to be changed to remove leading zeros
constexpr size_t kSize = 128;
char buffer[kSize];
DiscoveryFilter filter;
// Long tests
filter.type = DiscoveryFilterType::kLong;
filter.code = 3;
NL_TEST_ASSERT(inSuite, MakeCommissionableNodeServiceTypeName(buffer, sizeof(buffer), filter) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, strcmp(buffer, "_L0003._sub._chipc") == 0);


This comment was generated by todo based on a TODO comment in e6502ef in #7633. cc @cecille.

@todo
Copy link

todo bot commented Jun 15, 2021

add tests for longer vendor codes once the leading zero issue is fixed.

// TODO:add tests for longer vendor codes once the leading zero issue is fixed.
// Device Type tests
filter.type = DiscoveryFilterType::kDeviceType;
filter.code = 3;
NL_TEST_ASSERT(inSuite, MakeCommissionableNodeServiceTypeName(buffer, sizeof(buffer), filter) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, strcmp(buffer, "_T003._sub._chipc") == 0);
// TODO: Add tests for longer device types once the leadng zero issue is fixed.
// Commisioning mode tests
filter.type = DiscoveryFilterType::kCommissioningMode;


This comment was generated by todo based on a TODO comment in e6502ef in #7633. cc @cecille.

@todo
Copy link

todo bot commented Jun 15, 2021

Add tests for longer device types once the leadng zero issue is fixed.

// TODO: Add tests for longer device types once the leadng zero issue is fixed.
// Commisioning mode tests
filter.type = DiscoveryFilterType::kCommissioningMode;
filter.code = 0;
NL_TEST_ASSERT(inSuite, MakeCommissionableNodeServiceTypeName(buffer, sizeof(buffer), filter) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, strcmp(buffer, "_C0._sub._chipc") == 0);
filter.code = 1;
NL_TEST_ASSERT(inSuite, MakeCommissionableNodeServiceTypeName(buffer, sizeof(buffer), filter) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, strcmp(buffer, "_C1._sub._chipc") == 0);
filter.code = 2; // only or or 1 allwoed


This comment was generated by todo based on a TODO comment in e6502ef in #7633. cc @cecille.

@github-actions
Copy link

Size increase report for "gn_qpg6100-example-build" from b0eeffc

File Section File VM
chip-qpg6100-lighting-example.out .text 1264 1264
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
.debug_info,0,22609
.debug_line,0,6177
.debug_loc,0,5401
.debug_str,0,3675
.debug_abbrev,0,3598
.text,1264,1264
.strtab,0,1171
.debug_ranges,0,872
.symtab,0,720
.debug_frame,0,668
.debug_aranges,0,200
.shstrtab,0,1
[Unmapped],0,-1264

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'


@github-actions
Copy link

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

File Section File VM
chip-lock.elf text 1256 1256
chip-lock.elf rodata 16 16
chip-lock.elf device_handles 8 8
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,32593
.debug_line,0,6791
.debug_loc,0,4714
.debug_abbrev,0,4316
.debug_str,0,3554
text,1256,1256
.strtab,0,1165
.debug_ranges,0,816
.symtab,0,752
.debug_frame,0,664
.debug_aranges,0,200
rodata,16,16
device_handles,8,8
.shstrtab,0,-1

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

sections,vmsize,filesize


Copy link
Contributor

@Damian-Nordic Damian-Nordic left a comment

Choose a reason for hiding this comment

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

some nits, but looking good overall.

src/lib/mdns/Discovery_ImplPlatform.cpp Outdated Show resolved Hide resolved
src/lib/mdns/ServiceNaming.cpp Outdated Show resolved Hide resolved
src/lib/mdns/tests/TestServiceNaming.cpp Show resolved Hide resolved
@boring-cyborg boring-cyborg bot added the inet label Jun 15, 2021
@pullapprove pullapprove bot requested a review from pan-apple June 15, 2021 17:50
@woody-apple
Copy link
Contributor

@woody-apple
Copy link
Contributor

/rebase

@@ -339,6 +340,8 @@ static void TestDNSResolution_NoHostRecord(nlTestSuite * testSuite, void * testC
false
}
);
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: align #endif with above?

Not sure if this would be fighting the autoformatter :( looks odd though.

@woody-apple woody-apple merged commit 2fbc3fd into project-chip:master Jun 16, 2021
@cecille cecille deleted the mdns_platform_impl_for_commissionable branch July 23, 2021 19:34
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* Platform impl for commissionable node discovery

* Address review comments from Damian
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.

Commissionable node discovery for platform
4 participants