Skip to content

Refactors mdns-discovery to better handle add and remove events #14

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

Merged
merged 7 commits into from
Oct 27, 2021

Conversation

silvanocerza
Copy link
Contributor

Right now mdns-discovery has some issues caused by the mDNS library in use and by technical limitations of the mDNS protocol.

Because of the way the mDNS protocol works we have no way of knowing if a device has been disconnected from the network, only when it's connected/discovered.

The best way to handle this is to use TTLs and send a remove event when the TTL is reached. But the library we're using github.com/brutella/dnssd doesn't handle TTLs correctly so we never receives disconnections events from it. Also we don't have much granular control of the discovery process with this library.

I changed the library used to github.com/hashicorp/mdns and the internal behaviour of the discovery process.

Every 15 seconds we query the network to know if there are boards connected, each discovered port is cached with a TTL of 60 seconds and an add event is sent to consumers.
Each time we find a board we first check if it's cached, if it is we renew the TTL to 60 seconds. No add event is sent in this case.
When the TTL ends the board is removed from cache and a remove event is sent to consumers.

@silvanocerza silvanocerza added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Oct 27, 2021
@silvanocerza silvanocerza requested a review from a team October 27, 2021 10:31
@silvanocerza silvanocerza self-assigned this Oct 27, 2021
@facchinm
Copy link
Member

When touching this library, make sure not to reintroduce some bugs long fixed in the java IDE (like to ones linked here arduino/Arduino#5879 ).
The main actions taken to bring down cpu usage were:

  • avoid polling at all costs
  • keep track of online / offline network interfaces (eg. don't spam with requests interfaces without an IP / virtual ones)

Copy link

@ubidefeo ubidefeo left a comment

Choose a reason for hiding this comment

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

Tested on Mac OS (on which it was problematic) and it works.
Even sees other devices on my network, which is a welcome change ✌🏼

@silvanocerza
Copy link
Contributor Author

@facchinm no worries, the CPU usage is minimal, it never went more than 0.8% for me on Linux.

@facchinm
Copy link
Member

I'd suggest to test on a Windows pc with 5 network interfaces but it's not so easy to find 😅

@silvanocerza silvanocerza merged commit 94b56ce into main Oct 27, 2021
@silvanocerza silvanocerza deleted the scerza/refactor branch October 27, 2021 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants