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

Controller: Add other mdns filters #7659

Merged
merged 5 commits into from
Jun 23, 2021

Conversation

cecille
Copy link
Contributor

@cecille cecille commented Jun 15, 2021

Problem

Mdns exposes serveral subtypes that can be used to narrow the dnssd
search parameters. Previously, the only exposed parameter was the
long discriminator, because it was used in the QR code.
Fixes #6691

Change overview

Adding remaining commisisonable nodes discovery filters.
Also changes the controller API function to take the filter structure
directly, and uses the python script binding to set the filters
as required.

Testing

Tested with M5 in commissionable mode / chip-device-ctrl

  • discover usage (discover with no args)
  • discover -all
  • discover -qr with good and bad qr
  • all variants with good and bad values
    verified all args with correct values return data, all args with bad arguments return no results.

Mdns exposes serveral subtypes that can be used to narrow the dnssd
search parameters. Previously, the only exposed parameter was the
long discriminator, because it was used in the QR code. Adding others.
Also changes the controller API function to take the filter structure
directly, and uses the python script binding to set the filters
as required.

Test:
Tested with M5 in commissionable mode / chip-device-ctrl
- discover usage (discover with no args)
- discover -all
- discover -qr with good and bad qr
- all variants with good and bad values
verified all args with correct values return data, all args with bad arguments return no results.
@todo
Copy link

todo bot commented Jun 15, 2021

(cecille): I suppose we could make this a command line arg. Or Add a callback when

# TODO(cecille): I suppose we could make this a command line arg. Or Add a callback when
# x number of responses are received. For now, just 2 seconds. We can all wait that long.
print("Waiting for device responses...")
time.sleep(2)
def do_discover(self, line):
"""
discover -qr qrcode
discover -all
discover -l long_discriminator
discover -s short_discriminator


This comment was generated by todo based on a TODO comment in 6852fed in #7659. cc @cecille.

# x number of responses are received. For now, just 2 seconds. We can all wait that long.
print("Waiting for device responses...")
time.sleep(2)

def do_discover(self, line):
Copy link
Contributor

Choose a reason for hiding this comment

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

Although not merged, #7432 has introduced use of argparse to remove the need for all this boilerplate just to define the args interface.

This PR is expanding the number of arguments for do_discover from 2 to 8, so is nearly a complete refactor. Given this, would you consider using argparse as well here? It's just a whole lot easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed over in next commit. I'm not going to tackle any other refactors in this PR. I suppose this is ok - it's a bit longer, but does essentially the same thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

@woody-apple
Copy link
Contributor

/rebase

@woody-apple
Copy link
Contributor

/rebase

@woody-apple
Copy link
Contributor

/rebase

@cecille
Copy link
Contributor Author

cecille commented Jun 23, 2021

/rebase

@woody-apple woody-apple merged commit 8a53528 into project-chip:master Jun 23, 2021
@cecille cecille deleted the mdns_more_filters 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
* Controller: Add other mdns filters

Mdns exposes serveral subtypes that can be used to narrow the dnssd
search parameters. Previously, the only exposed parameter was the
long discriminator, because it was used in the QR code. Adding others.
Also changes the controller API function to take the filter structure
directly, and uses the python script binding to set the filters
as required.

Test:
Tested with M5 in commissionable mode / chip-device-ctrl
- discover usage (discover with no args)
- discover -all
- discover -qr with good and bad qr
- all variants with good and bad values
verified all args with correct values return data, all args with bad arguments return no results.

* Fix function name in connect qr

* use argparse.
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.

Add remaining commissionable node subtype filters
5 participants