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

Discovery of Play:3 crashes #4

Open
altery opened this issue Nov 29, 2012 · 9 comments
Open

Discovery of Play:3 crashes #4

altery opened this issue Nov 29, 2012 · 9 comments

Comments

@altery
Copy link

altery commented Nov 29, 2012

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  
@SR-G
Copy link
Owner

SR-G commented Nov 30, 2012

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
Copy link
Owner

SR-G commented Nov 30, 2012

Code ok on my side, just commited

@altery
Copy link
Author

altery commented Dec 1, 2012

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
Copy link
Author

altery commented Dec 1, 2012

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.

@fhubin
Copy link

fhubin commented Dec 4, 2012

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;
    }
}

@altery
Copy link
Author

altery commented Dec 21, 2012

Any progress on this?

@fhubin
Copy link

fhubin commented Jan 7, 2013

@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?

@altery
Copy link
Author

altery commented Jan 7, 2013

@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.

@fhubin
Copy link

fhubin commented Jan 7, 2013

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

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

No branches or pull requests

3 participants