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

MulticastService optimizations #35

Closed
wants to merge 1 commit into from
Closed

MulticastService optimizations #35

wants to merge 1 commit into from

Conversation

eshvatskyi
Copy link
Contributor

  • fixed AnswersContainsAdditionalRecords functionality
  • fixed using MulticastService with multiple NICs
  • removed exploring networks by timers instead used NetworkChange.NetworkAddressChanged

@richardschneider
Copy link
Owner

It appears that this PR is not based on the latest bits. The recently sent logic has been removed!

You will need to rebase the PR.

@richardschneider
Copy link
Owner

@eshvatskyi Good find on NetworkChangedEvent. Is it raised when the laptop wakes up from a sleep?

@eshvatskyi
Copy link
Contributor Author

@eshvatskyi Good find on NetworkChangedEvent. Is it raised when the laptop wakes up from a sleep?

yes, it is

@eshvatskyi
Copy link
Contributor Author

It appears that this PR is not based on the latest bits. The recently sent logic has been removed!

You will need to rebase the PR.

done. Also please check GetIPAddresses method, looks like it should be default return only InterNetwork adressses

@eshvatskyi
Copy link
Contributor Author

@richardschneider I've tried to fix all timeout issues from test, but looks like this is non windows specific issues. Please let me know if you can go thought with me

 - fixed using MulticastService with multiple NICs
 - removed exploring networks by times, instead used NetworkChange.NetworkAddressChanged
@richardschneider
Copy link
Owner

Lots of issues here. I especially don't like breaking the Interface (removing IDisposable) and changing all tests. This means all dependent packages are broken.

I feel that maintaining multiple senders is wrong. Multiple NICs are already suppported (see #15 and #12). If you disagree, please raise an issue describing the problem.

I suggest that you proceeded by creating two new PRs

  • fix AnswersContainsAdditionalRecords functionality
  • use NetworkAddressChanged event

I do appreciate the work you are doing. Please continue.

@eshvatskyi
Copy link
Contributor Author

np, but it's not working with multiple nic as expected.
when you sending query it's not sent to all subnets, but to first suitable.
sender.MulticastLoopback = true;
I dont nkow why this is used, this is actually duplicated query to loopback (good for testing not useful in subnets)

@richardschneider
Copy link
Owner

sender.MulticastLoopback = true;
I dont nkow why this is used, this is actually duplicated query to loopback (good for testing not useful in subnets)

MulticastLoopback has nothing to do with the loopback interface. It allows a sender to also receive its sent message. See the docs, A big feature of mDNS is passive monitoring of messages. With loopback, the sending app can respond to its own query without having to write special code to deal with a query from itsself or another.

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.

2 participants