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

Multi-interface support. #38

Closed
wants to merge 9 commits into from
Closed

Conversation

bnielsen1965
Copy link
Contributor

I have been running this forked version of multicast-dns for a couple of months with no issues. It provides support for multiple network interface listening and queries. The other pull request that addresses this need is stalled so I thought I should provide the opportunity to merge this fork if the code changes are acceptable.

This change allows an array of interfaces to be specified in the options when creating an instance of multicast-dns. All specified interfaces will then be used to listen and a socket will be opened on each interface to send out queries.

@TekSiDoT
Copy link

Would love to see this merged!

@mafintosh
Copy link
Owner

This is great. I'll try to get this merged asap.

@bnielsen1965
Copy link
Contributor Author

@mafintosh Do you need me to help address the conflict?

I think the conflict is related to pull request 34 which has been merged...
#34

I'm sure I merged that pull request into my fork but I think I also made an additional change which may be the source of the conflict.

The change in pull request #34 added a try/catch statement that emits an error if the addMembership fails, after the merge I change it from emitting an error to emitting a warning because the error will cause older apps using the module to crash it they do not catch the error. A warning seemed more forgiving.

So I think the conflict can be resolved with a merge or I can re-fork the existing repo, add the multi-interface changes, then create a new pull request. Your option, just let me know and I'll do what I can to help out.

@panta panta mentioned this pull request Jun 14, 2017
@mafintosh
Copy link
Owner

@bnielsen1965 apologise for being slow. if you wanna fix up the conflict i'll merge it :)

@bnielsen1965
Copy link
Contributor Author

I'm closing this pull request as it is superseded by #42

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.

4 participants