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

[cli] update borderagent discover to allow network interface selection for mDNS binding #259

Merged
merged 13 commits into from
Apr 29, 2024

Conversation

ZhangLe2016
Copy link
Contributor

@ZhangLe2016 ZhangLe2016 commented Apr 12, 2024

Within the OTBR practice application, the BR host will have several interfaces. When users want to discover the border agent using mDNS, they'll need to choose the specific interface the border agent is bound to.

This change introduces a new option for the CLI command borderagent dicover. Users can now specify a network interface using the syntax borderagent discover <timeout> <network interface>. The chosen interface will be used for mDNS binding through socket options.

…ce selection for mDNS binding.

Within the OTBR practice application, the BR host will have several
interfaces. When users want to discover the border agent using mDNS,
they'll need to choose the specific interface the border agent is bound
to.

This change introduces a new option for the CLI command `borderagent
dicover`. Users can now specify a network interface using the syntax
`borderagent discovery <timeout> <network interface>`. The chosen
interface will be used for mDNS binding through socket options.
@ZhangLe2016 ZhangLe2016 changed the title [cli] Update command "borderagent discovery" to allow network interface selection for mDNS binding. [cli] Update command "borderagent discover" to allow network interface selection for mDNS binding. Apr 12, 2024
Copy link
Member

@wgtdkp wgtdkp left a comment

Choose a reason for hiding this comment

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

See also my comments in #260

src/app/cli/interpreter.cpp Outdated Show resolved Hide resolved
…ce selection for mDNS binding.

Within the OTBR practice application, the BR host will have several
interfaces. When users want to discover the border agent using mDNS,
they'll need to choose the specific interface the border agent is bound
to.

This change introduces a new option for the CLI command `borderagent
dicover`. Users can now specify a network interface using the syntax
`borderagent discovery <timeout> <network interface>`. The chosen
interface will be used for mDNS binding through socket options.
Copy link
Member

@wgtdkp wgtdkp left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! 👍

Just a few nits.

src/app/br_discover.hpp Outdated Show resolved Hide resolved
src/app/br_discover.cpp Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Apr 24, 2024

Codecov Report

Attention: Patch coverage is 70.83333% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 72.91%. Comparing base (cc2edbf) to head (28dbb7b).
Report is 4 commits behind head on main.

❗ Current head 28dbb7b differs from pull request most recent head af38b65. Consider uploading reports for the commit af38b65 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #259      +/-   ##
==========================================
- Coverage   72.98%   72.91%   -0.07%     
==========================================
  Files          72       72              
  Lines        7477     7496      +19     
==========================================
+ Hits         5457     5466       +9     
- Misses       2020     2030      +10     
Files Coverage Δ
src/app/border_agent_functions_mock.cpp 100.00% <100.00%> (ø)
src/app/border_agent_functions_mock.hpp 100.00% <100.00%> (ø)
src/app/br_discover.cpp 85.71% <66.66%> (-3.76%) ⬇️
src/app/cli/interpreter.cpp 79.52% <66.66%> (-0.38%) ⬇️

... and 2 files with indirect coverage changes

src/app/br_discover.cpp Outdated Show resolved Hide resolved
@ZhangLe2016 ZhangLe2016 requested a review from wgtdkp April 25, 2024 04:39
@ZhangLe2016 ZhangLe2016 marked this pull request as ready for review April 25, 2024 09:21
src/app/br_discover.cpp Outdated Show resolved Hide resolved
src/app/br_discover.cpp Outdated Show resolved Hide resolved
@wgtdkp wgtdkp self-requested a review April 25, 2024 12:54
@ZhangLe2016 ZhangLe2016 requested a review from jwhui April 26, 2024 05:08
@jwhui jwhui changed the title [cli] Update command "borderagent discover" to allow network interface selection for mDNS binding. [cli] update borderagent discover to allow network interface selection for mDNS binding Apr 29, 2024
@jwhui jwhui added the cli label Apr 29, 2024
@jwhui jwhui merged commit 3d33a42 into openthread:main Apr 29, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants