vmichalak / sonos-controller

Java API for controlling SONOS players
MIT License
43 stars 9 forks source link

Inconsistent discovery #30

Closed anton-johansson closed 6 years ago

anton-johansson commented 6 years ago

I'm running the following code:

List<SonosDevice> devices = SonosDiscovery.discover();
for (SonosDevice device : devices)
{
    SonosSpeakerInfo speakerInfo = device.getSpeakerInfo();
    System.out.println("Found speaker at " + speakerInfo.getIpAddress() + ": " + speakerInfo.getZoneName());
}

I get different results every time I run it. Four examples:

Found speaker at 192.168.1.10: A
Found speaker at 192.168.1.11: B
Found speaker at 192.168.1.12: C
Found speaker at 192.168.1.13: D
Found speaker at 192.168.1.12: C
Found speaker at 192.168.1.11: B
Found speaker at 192.168.1.10: A
Found speaker at 192.168.1.13: D
Found speaker at 192.168.1.12: C
Found speaker at 192.168.1.10: A
Found speaker at 192.168.9.10: A
Found speaker at 192.168.9.13: D

As you can see, I'm not always getting all devices. Do you know what the reason of this could be?

anton-johansson commented 6 years ago

Could it be the one second timeout?

SSDPClient.discover(1000, "urn:schemas-upnp-org:device:ZonePlayer:1");
vmichalak commented 6 years ago

Your problem is related with how SSDP works. Extend the timeout is not a good solution (all users doesn't need to wait 1s and for some people 1s is not enough).

I think it could be better to purpose an optional scanDuration parameter.

anton-johansson commented 6 years ago

Yeah, sounds like a good idea. I can try it out, unless you're already working on it? :)

vmichalak commented 6 years ago

If you want implement it, go ! :)

anton-johansson commented 6 years ago

I messed around with some timeouts and I still experienced the issue with 10 seconds timeout, but not as often. Not sure if keep extending the timeout is a good solution.

vmichalak commented 6 years ago

It's perhaps a network issue on your side.

anton-johansson commented 6 years ago

Yeah, could be. I was at my office, where there's quite a big network. I'll submit a PR tomorrow that allows passing in the scan duration. However, I might not use it myself, so you can decide whether or not you want the functionality.

I decided that I will manually add Sonos devices when configuring my system. I will use the discovery as a help, only. For example:

> sonos:discover
"Kitchen" at 192.168.1.1
"Playbar in living room" at 192.168.1.2

> sonos:add --name kitchen 192.168.1.1
vmichalak commented 6 years ago

It's a solution, I wait you PR with attention :)

vmichalak commented 6 years ago

Merged PR.