sonos-java icon indicating copy to clipboard operation
sonos-java copied to clipboard

Discovery of Play:3 crashes

Open altery opened this issue 13 years ago • 9 comments

When sonos-java discovers a play:3 on the network it fails to add it as zone because of a NPE. Specifically, the exception occurs when creating the AudioInService -> AbstractService.findService() returns null, which gets passed directly to the constructor of the AudioInService. As the play:3 does not have an audio in, I assume it is an expected behaviour that a service lookup may fail, but sonos-java doesn't handle that case correctly.

Here's the stacktrace:

Daemon Thread [SonosControllerThread] (Suspended (exception NullPointerException))  
    owns: ZonePlayerModel  (id=42)  
    AudioInService(AbstractService).<init>(UpnpService, Service, String) line: 128  
    AudioInService.<init>(UpnpService, Service) line: 41    
    ZonePlayer.<init>(UpnpService, RemoteDevice) line: 112  
    CLIController(AbstractController).addZonePlayer(RemoteDevice) line: 110 
    JavaController$2$1.run() line: 118  
    ThreadPoolExecutor$Worker.runTask(Runnable) line: 886   
    ThreadPoolExecutor$Worker.run() line: 908   
    Thread.run() line: 680  

altery avatar Nov 29 '12 23:11 altery

Ok i've corrected that one on my side. I'll try it at home tonight on my Sonos boxes to test there are no regressions, and after that i'll commit it. (i only have two ZonePlayer and one Sonos S:5, so i won't be able to check the correct behavior on Sonos S:3 boxes)

SR-G avatar Nov 30 '12 11:11 SR-G

Code ok on my side, just commited

SR-G avatar Nov 30 '12 22:11 SR-G

Unfortunately, this does not help. The call to the constructor of AbstractService with the value null is still performed, which leads to the very same exception. I created a patch that fixes the issue for my case using a more generic approach (undoing most of your changes, sorry). Please see the following gist:

https://gist.github.com/4182132

Would be great if you could try it out and integrate it into master if no regressions occur.

altery avatar Dec 01 '12 13:12 altery

On, I just noticed that the workaround for another issue made it into the patch. You may want to exclude the diff for CLIController... Sorry about that.

altery avatar Dec 01 '12 13:12 altery

Hi guys,

I've stumbled onto this problem as well. To fix it, I let audioIn be null in ZonePlayer and throw a newly created AudioInUnavailableException exception when trying to retrieve it through getAudioInService. dispose() needs to be modified as well to avoid an NPE. I'm not sure if this way of fixing the issue respect the philosophy but it works fine.

In ZonePlayer:

public AbstractAudioInService getAudioInService () throws AudioInUnavailableException {

    if(audioIn == null) {
        throw new AudioInUnavailableException(MessageFormat.format("No audio-in available for ${0}", getId()));
    }

    return audioIn;
}

public void dispose() {
    mediaServer.dispose();
    mediaRenderer.dispose();
    alarm.dispose();

    if(audioIn != null) {
        audioIn.dispose();
    }

    deviceProperties.dispose();
    systemProperties.dispose();
    zoneGroupTopology.dispose();
    zoneGroupManagement.dispose();
}

In AbstractAudioService:

public static AbstractAudioInService buildAudioInService(final UpnpService upnpService, final RemoteDevice dev) {
    final Service audioService = AbstractService.findService(dev, ZonePlayerConstants.SONOS_SERVICE_AUDIO_IN);
    if (audioService != null) {
        return new AudioInService(upnpService, audioService);
    } else {
        return null;
    }
}

fhubin avatar Dec 04 '12 17:12 fhubin

Any progress on this?

altery avatar Dec 21 '12 11:12 altery

@altery would you be interested in a fork? I'm working on a home automation projet (http://www.idomos.org) and I would like to incorporate Sonos in it. We can perhaps join forces on the Sonos bit?

fhubin avatar Jan 07 '13 07:01 fhubin

@fhubin that would be interesting. I just started a new project based on the research of this work (https://github.com/altery/s4m), but if I could incorporate my requirements in a greater context I'd be glad to help.

altery avatar Jan 07 '13 07:01 altery

@altery Ok. Let me have a look at your project tonight and I'll get back to you.

fhubin avatar Jan 07 '13 08:01 fhubin