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

Add support for "commisionable" discovery advertisement #4273

Conversation

arunbharadwaj
Copy link
Contributor

Problem

We need support for "commisionable" discovery advertisement according to Discovery spec for CHIP

Summary of Changes

This adds support for "commisionable" discovery advertisement.

This builds on top of PR #4200 which adds "commisioning" and "operational" discovery advertisements
This adds the Advertise() function in lib/mdns/Advertiser_ImplMinimalMdns.cpp to advertise service as _chipd._udp while running in commisionable mode.
This also adds the class CommisionableAdvertisingParameters which additionally contains pairingInstr and pairingHint
Finally, this adds some test code in the example/minimal-mdns/advertiser.cpp to test this mode.

@CLAassistant
Copy link

CLAassistant commented Jan 5, 2021

CLA assistant check
All committers have signed the CLA.

@rwalker-apple
Copy link
Contributor

This has the commits from #4200, right?

Would you mind re-requesting review once that's landed and this PR is rebased so we can examine your changes in isolation?

Copy link
Contributor

@rwalker-apple rwalker-apple left a comment

Choose a reason for hiding this comment

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

.

@arunbharadwaj
Copy link
Contributor Author

This has the commits from #4200, right?

Would you mind re-requesting review once that's landed and this PR is rebased so we can examine your changes in isolation?

Yes, I will rebase this PR once #4200 is merged.

For now, posted here to get some early feedback.

@andy31415
Copy link
Contributor

@arunbharadwaj - I believe #4200 has been merged and this should be ready for rebase/merge

@arunbharadwaj
Copy link
Contributor Author

Created a new PR #4337 as there were too many merge conflicts after rebasing. Closing this PR.

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