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.

Sign up for GitHub

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

Jump to bottom

[Darwin] Add the ability to browse operational nodes with the device scanner #27940

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[Darwin] Add the ability to browse operational nodes with the device scanner #27940

wants to merge 4 commits into from
+361 −100

Conversation

vivien-apple
Copy link
Contributor

Problem

There is no underlying API for the DnssdBrowseDelegate to resolve operational nodes.
This PR add such supports for the darwin backend as well as a command in chip-tool for it.

This is an attempt to get #26718 to move forward to the DnssdBrowseDelegate. The benefit of it is that is allow to monitor nodes for long lived operations, such as addition and removals.
@vivien-apple vivien-apple self-assigned this Jul 13, 2023
@mergify
Copy link

mergify bot commented Jul 25, 2023

⚠️ The sha of the head commit of this PR conflicts with #28258. Mergify cannot evaluate rules on this PR. ⚠️
@vivien-apple
Copy link
Contributor Author

I have updated a little bit the code from src/platform/Darwin. The protocol type was not stored in the DnssdService properly and that was causing issues.
@mergify
Copy link

mergify bot commented Jul 25, 2023

⚠️ The sha of the head commit of this PR conflicts with #28258. Mergify cannot evaluate rules on this PR. ⚠️
@vivien-apple vivien-apple mentioned this pull request Jul 26, 2023
Open
@stale
Copy link

stale bot commented Oct 15, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
@stale stale bot added the stale Stale issue or PR label Oct 15, 2023
@bzbarsky-apple bzbarsky-apple removed the stale Stale issue or PR label Oct 16, 2023
@woody-apple woody-apple added this to the 1.3 Feature Complete milestone Oct 19, 2023
@pullapprove pullapprove bot requested a review from turon November 8, 2023 16:26
andy31415
andy31415 requested changes Jan 26, 2024
View reviewed changes
Copy link
Contributor

@andy31415 andy31415 left a comment

Choose a reason for hiding this comment

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

Changes requested - I am afraid this looks to bypass comments in #26718 by creating a new PR.

Overall I need to understand why we would like to browse operational nodes - that seems to create extra traffic for nodes that are inactive.

Please add some description of use case and explanation how we measure "success". It also needs unit tests (or I guess integration tests in this case because of the level it is at) and implementations in more than just darwin since it is a global thing - avahi and minmdns seems needed. I am worried about minmdns overhead and complexity.
@bzbarsky-apple
Copy link
Contributor

that seems to create extra traffic for nodes that are inactive.

@andy31415 I don't understand this part. What definition of "inactive" are we using here?

Fundamentally, browsing for operational nodes is a way to get notified when a node that was not on the network starts being on the network.
@stale Stale
Copy link

stale bot commented Apr 22, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
@stale stale bot added the stale Stale issue or PR label Apr 22, 2024
@bzbarsky-apple bzbarsky-apple removed the stale Stale issue or PR label Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
darwin examples lib platform review - pending tools
4 participants
@vivien-apple @bzbarsky-apple @andy31415 @woody-apple