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 subtypes #31

Closed

Conversation

ssnover
Copy link
Contributor

@ssnover ssnover commented Sep 13, 2023

Resolves #30

Tested, here's a log snippet:

[2023-09-13T00:01:24Z DEBUG zeroconf::linux::service] Registering service: AvahiMdnsService { client: None, poll: None, context: AvahiServiceContext { name: None, kind: "_matterc._udp", port: 5540, group: None } }
[2023-09-13T00:01:24Z DEBUG zeroconf::linux::service] Creating group
[2023-09-13T00:01:24Z DEBUG zeroconf::linux::service] Adding service: _matterc._udp
[2023-09-13T00:01:24Z DEBUG zeroconf::linux::service] Adding service subtype: _L250._sub._matterc._udp
[2023-09-13T00:01:24Z DEBUG zeroconf::linux::service] Adding service subtype: _S0._sub._matterc._udp
[2023-09-13T00:01:25Z DEBUG zeroconf::linux::service] Group established

And from avahi-browse (snipped extraneous info):

$ avahi-browse -t _matterc._udp -r
...
= wlp2s0 IPv6 pop-os                                        _matterc._udp        local
   hostname = [pop-os.local]
   address = [fd4d:e11e:2aab:1:c8b5:684f:3826:6afe]
   port = [5540]
   txt = ["PI=" "PH=33" "SAI=300" "SII=5000" "VP=65521+32768" "DN=TestOnOff" "CM=1" "D=250"]
...

Copy link
Owner

@windy1 windy1 left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution, looks good overall just have a couple minor requests. 👍

zeroconf/src/linux/service.rs Outdated Show resolved Hide resolved
zeroconf/src/linux/entry_group.rs Show resolved Hide resolved
zeroconf/src/linux/service.rs Outdated Show resolved Hide resolved
@ssnover ssnover requested a review from windy1 September 17, 2023 22:14
@ssnover
Copy link
Contributor Author

ssnover commented Sep 19, 2023

By the way, I'd recommend maybe pinning the Rust version used for clippy linting. In order for CI to pass I had to fix some warning level clippy lints that were introduced in the newest version.

@windy1
Copy link
Owner

windy1 commented Sep 21, 2023

Hey there - I've been looking into this PR a bit deeper and there are a couple issues I currently see with it:

  1. We need to implement this functionality in Bonjour too so that we maintain feature parity between Avahi and Bonjour
  2. The implementation of SubType is specific to Avahi. In Bonjour, you register subtypes by appending them to the end of the normal type in a comma-separated list (so in the test you've modified it would be _http._tcp,_printer). Therefore, formatting to Avahi's specifications in ServiceType, which is intended to be cross-platform, is not appropriate. This formatting should be done independently from the ServiceType implementation.

Bonjour reference: https://developer.apple.com/documentation/dnssd/1804733-dnsserviceregister?language=objc

I appreciate you may not have the hardware to implement the Bonjour side of this so I've started a branch to amend your changes. Once I am happy with it I will submit a PR to this branch and then we can merge into main. Thanks for your patience.

@windy1
Copy link
Owner

windy1 commented Sep 23, 2023

Superseded by #32

@windy1 windy1 closed this Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Service subtypes do not appear to be registered correctly
2 participants