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

fix: multiple services by one host get now indexed by the SRV name, not the PTR name. #225

Merged

Conversation

schuenep
Copy link

This allows to announce and find multiple services of the same type e.g. _http._tcp by one server.

Fixes #198

@schuenep schuenep force-pushed the fix/multiple_services_same_host branch from 19e91a2 to 7828e2f Compare January 13, 2022 13:20
@schuenep schuenep changed the title fix: multiple services by one host get now indexed by the SRV name, not the PTR name. WIP: fix: multiple services by one host get now indexed by the SRV name, not the PTR name. Jan 13, 2022
@schuenep
Copy link
Author

schuenep commented Jan 13, 2022

Unfortunately I needed to change a little bit more, as the Name is used in the dictionary services in ZeroconfRecord.cs as the key and this key was used in multiple locations when finding a service of a specific type.
This also adds a breaking change to the public API. E.g. in our case we used the key on the return value of ResolveAsync.
If one wants the same functionality instead of service.key now service.Value.Name must be used.

@schuenep schuenep force-pushed the fix/multiple_services_same_host branch from f43d994 to 03a3a78 Compare January 13, 2022 16:02
@schuenep schuenep changed the title WIP: fix: multiple services by one host get now indexed by the SRV name, not the PTR name. fix: multiple services by one host get now indexed by the SRV name, not the PTR name. Jan 13, 2022
@clairernovotny
Copy link
Collaborator

Few things:

  1. Can you please review the readme.md to see if anything needs to be changed in the sample code given the updates?
  2. Please add a note in the readme about this change and how to mitigate
  3. Please bump the version.json to 3.6 as there's a minor break here

@schuenep schuenep force-pushed the fix/multiple_services_same_host branch from 3bc238d to e5dde0d Compare January 14, 2022 12:06
@schuenep
Copy link
Author

Few things:

  1. Can you please review the readme.md to see if anything needs to be changed in the sample code given the updates?
  2. Please add a note in the readme about this change and how to mitigate
  3. Please bump the version.json to 3.6 as there's a minor break here
  1. I did not find anything that needs to be changed
  2. Done, please have a look
  3. Done

Also I did search for additional usages of the dictonary .Key inside the project and replaced them with .Value.Name. Please have a look.

@schuenep
Copy link
Author

Hi, anything else I need to fix?

@clairernovotny
Copy link
Collaborator

I don't think so; just haven't had a chance to test it locally. Will try to get that done soon.

@clairernovotny clairernovotny merged commit baa3eb8 into novotnyllc:main Feb 4, 2022
@schuenep schuenep deleted the fix/multiple_services_same_host branch February 10, 2022 10:02
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.

Zeroconf.ResolveAsync does not find multiple service advertised from the same host
2 participants