Logical Device Support

[stephen]

cc/ @bencevans

Changes:

• Changed Listener class to think in terms of "handlers" instead of "endpoints." This makes more sense because you might have multiple handlers for a given endpoints' events, so "client" code should think about handlers logically. I don't believe any previous code used these, but it could technically be considered a breaking change (API changed addService/removeService to be prefixed with _.
• Added LogicalDevice class. Takes in an array of Sonos devices that can be managed together (i.e. forms a group or stereo pair). This class currently supports group change for volume. The API between a LogicalDevice and regular Sonos device should be identical. (Commands that are only addressed to the coordinator (e.g. play/pause) are simply passed down to the underlying Sonos prototype.
• Added a volumeListener class to keep internal state on a Sonos object. Used by LogicalDevice to do volume changes (but not on other methods).

(Closes #43)

[stephen]

@bencevans or @mrose17, could one of you review this code?

There's also a question of whether or not this works on stereo pairs - I unfortunately don't have one to test with and have been working with a grouped Play:1/Play:3.

[bencevans]

I can view code but still don't have a Sonos where I am now so won't get to test out till later this month.

[stephen]

@bencevans - kk. I've been testing against real devices, and perhaps @mrose17 can help out as well. Separately, a pure code review would be great if you have time.

[bencevans]

As discussed starting the listeners on initialisation

[stephen]

(oops - this should be initialize() instead of startListeners())

We previously discussed doing initialization for listeners automatically with search (meaning manually spun up Sonos instances would have to initialize()) on their own. Are you thinking we should do automatic initialization for any Sonos instance? (also, how? since a callback is needed - not sure if it's good practice to do that from a constructor.)

[bencevans]

Good point. I was thinking the client would be like a Sequelize
instance. (are you familiar?)

On Mon, 8 Dec, 2014 at 5:36 AM, Stephen Wan notifications@github.com
wrote:

In examples/volumeWatcher.js:

@@ -0,0 +1,8 @@
+var Sonos = require('../index').Sonos;
+
+var device = new Sonos(process.env.SONOS_HOST || '192.168.2.11');
+device.startListeners(function() {
(oops - this should be initialize() instead of startListeners())

We previously discussed doing initialization for listeners
automatically with search (meaning manually spun up Sonos instances
would have to initialize()) on their own. Are you thinking we should
do automatic initialization for any Sonos instance? (also, how? since
a callback is needed - not sure if it's good practice to do that from
a constructor.)

—
Reply to this email directly or view it on GitHub.

[stephen]

Hmm, iirc Sequelize does something like...

new Sequelize().initialize().etc();

Which relies on promises. The current API here is:

new Sonos().initialize(callback);

Which is more or less the same thing, except using callbacks, not promises.

My thinking is that we can automatically initialize for Sonos and LogicalDevice instances returned by a search()/"managed" sonos device, but leave calling initialize() up to client code if manually constructed (i.e. new Sonos().initialize(...).

[bencevans]

Okay. That's true, I don't know about you but I'd rather keep the callback & EventEmitter style rather than promises. Your thoughts?

[stephen]

Callbacks seem reasonable here.

[bencevans]

Just been for a peek through the code and it's looking delightful!

[stephen]

@bencevans, @mrose17 - Any other comments on this before merge?

[bencevans]

Not from me (although can't test). Any thoughts from @mrose17

[mrose17]

looks good to me. i won't be able to test until this evening though (i don't have a sonos here in the office... the new office will have one, hopefully)!

i say "commit"

[stephen]

TypeError: Cannot read property '0' of undefined
at /Users/stephen/git/node-sonos/lib/events/volumeListener.js:16:84
at Parser. (/Users/stephen/git/node-sonos/node_modules/xml2js/lib/xml2js.js:255:20)
at Parser.emit (events.js:95:17)

• When changing EQ settings. Should guard this listener from assumptions