bencevans / node-sonos

🔈 Sonos Media Player Interface/Client
https://www.npmjs.com/package/sonos
MIT License
703 stars 147 forks source link

ES6 classes for services plus some additional methods #469

Closed pascalopitz closed 4 years ago

pascalopitz commented 4 years ago

Hello,

I started going through a few of the additional bits I have implemented in my unofficial controller for linux, and started backporting them into node-sonos. As you can see I have a whole set of bits I have had to extend, to make several functionality bits work:

https://github.com/pascalopitz/unoffical-sonos-controller-for-linux/tree/master/src/ui/services/enhanced

In the past I have been crap at feeding back into this awesome library, and this time I want to do it right and make a meaningful contribution if I can.

My goal is to bit by bit move as much functionality back to node-sonos as possible and then cut it out of my code base, where that makes sense of course. I also am aiming to write a bit more detailed comments about some of the workflows I have discovered, i.e. the TruePlay setting.

Let me know if this is a) a welcome effort and b) if there's any parts that you'd rather not see materialise.

svrooij commented 4 years ago

Hi @pascalopitz maybe we can join forces? Recently I’ve been focusing on a typescript version of this library. You can check it out here. I’ve created a generator that actually generates most of the services by loading the discovery files.

And some services are manually extended, these extensions are merged in the services upon generating them. The ContentDirectoryService is a sample of a generated service with an extension on it.

My entire Sonos library is documented here.

From what I can see, you only changed from the old prototyped syntax to actual class syntax. And did some clever way to check the balans. From that point of view this PR looks totally acceptable. 👍

pascalopitz commented 4 years ago

Hi @svrooij ... I might well contribute to your library at some stage too, but for now, since I am heavily leaning on this project and a don't want to rewrite everything after I just made a pretty big set of changes, I just want to backport some stuff.

Yes, in this PR I have changed the prototype syntax and added the balance function. Also added some additional functions and doc blocks, i.e. the EnterConfigMode, ExitConfigMode and GetButtonState methods in DevicePropertes.

svrooij commented 4 years ago

Then @bencevans can just take this comment, run a small test, and approve these changes.

From what I can see, you only changed from the old prototyped syntax to actual class syntax. And did some clever way to check the balans. From that point of view this PR looks totally acceptable. 👍

Ps. I haven't run your code, I only browsed through, so running a small test might still be necessary.

bencevans commented 4 years ago

:tada: This PR is included in version 1.13.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

svrooij commented 4 years ago

@pascalopitz as we implemented all-contributors your image is also added the the readme