jishi / node-sonos-discovery

Simplified framework for Sonos built on node.js
MIT License
146 stars 75 forks source link

add option to listen on 0.0.0.0 instead on specific interfaces #40

Closed neophob closed 9 years ago

neophob commented 9 years ago

should fix #38

jishi commented 9 years ago

I could merge this, but eventually I would like to make it a bit more automatic. Using 0.0.0.0 would work for most users, so maybe that should be the default and if it doesn't find any players it could try and bind to specific IPs instead. That is of course a bigger rewrite.

neophob commented 9 years ago

Hmm other question, whats the reason to search on all interfaces anyway? whats the advantage?

another option which would be a bit more generic might be that the option contain the listening ip, this might be more generic? I don't know if the automatic solution solve the issue, maybe you start sonos discovery before the sonos device is online, then it wouln't work.

jishi commented 9 years ago

The reason was because in some scenarios, when binding to "all" interfaces, it only listened to all interfaces, but the outgoing traffic was only sent on a single interface, and not necessarily the "correct" one. Binding to all interfaces individually gave me the possibility to send on all interfaces and just act on the one that received a response. This was especially common when you had virtualization software installed on your computer.

One could also try and loop through the interfaces and search for players (and one for 0.0.0.0), but the discovery would be slower probably.

neophob commented 9 years ago

OK make sense. But what do you think about adding a specific IP address which could be used when the automatic discovery fails (like in my case)?

jishi commented 9 years ago

Well, a specific address won't work in your case since you have the same IP for tun0 and bond0. The 0.0.0.0 probably works because the OS itself will select the most appropriate interface to use (probably the one with the lowest metric), and in your case this is the best option (opposite to some other observations I've made).

I can merge this for now if you like, but I want to rewrite the logic later on. I also want to make this library Promise-based now that node 4.x.x has been released and there is native support for it.

neophob commented 9 years ago

Well 0.0.0.0 is a specific address too ;) I think thats fine, merge now and add more features later on.

BR and thanks