nfarina / homebridge-sonos

Sonos plugin for homebridge: https://github.com/nfarina/homebridge
155 stars 52 forks source link

Identify and register "coordinator" devices only in paired/grouped setups #8

Closed fwboettger closed 8 years ago

fwboettger commented 8 years ago

@nfarina i'd like to propose this change which i believe does resolve the issue “Intermittent error on startup, with paired Play 1's #4” (https://github.com/nfarina/homebridge-sonos/issues/4).

The code changes (based on code samples from node-sones) in index.js now identify the current “coordinator” sonos device for a respective zone (room) and register these as homekit accessories. Tested in my environment at home (playbar with sub and 2 play3 paired to a 5.1 surround setup in the living room, and several non-paired individual devices in other rooms).

adamstephenson commented 8 years ago

@fwboettger, this is a great improvement -- thanks! +1

Can you confirm testing with groups too? It looks like the coordinator designator would work in that situation as well -- as you also imply in your comment "...paired/grouped..."

Thanks again!

fwboettger commented 8 years ago

Dear @adamstephenson, thanks much! Confirm partial Tests for groups, but didnt have time to conduct extensively here as i've been flying of with the Family to a short vacation today.... In any case, pairing typically is pretty static once setup, grouping usually is way more dynamic i believe, and in Order to cater to this (Dynamic changes to the topology), i believe one needs to subscribe to respective events and react accordingly. Wanted to look into this separately after my return (seems to be a bit more complex).....

Nousemusic commented 8 years ago

@adamstephenson or @fwboettger can you help me install the changes?

Do I have to edit the home bridge-sonos plugin's index.js by hand or can I just reinstall the whole plugin via npm install -g homebridge-sonos?

Thank you for your help in advance.

fwboettger commented 8 years ago

@Nousemusic - for now you need to edit the index.js by hand i'm afraid. The pull request isn't yet been incorporated into the plugin's package, hence reinstalling would simply not include the changes....