ebaauw / homebridge-music

Homebridge plugin for iTunes with AirPlay speakers
Apache License 2.0
92 stars 7 forks source link

Fix app reopening on close #15

Closed spmcewen closed 4 years ago

spmcewen commented 4 years ago

First - thank you for starting this project, it provides the exact functionality I was hoping for.

I noticed that the state polling causes Music to reopen after you close it. So I added a check to determine if the Music app is running to the getState() and getSpeakerStates() functions. If it's not running some default JSON is returned to indicate the off state. I also added a new getInitialState() function which is called by homebridge on startup. This will force Music to open which should allow for an accurate list of airplay speakers to be found regardless if Music was open. This is working for me locally.

I tried compiling the scripts with the Makefile but am running into issues. I did add the new getInitialState() method to the existing iTunes, Airfoil and Music-Airfoil and EyeTV scripts. I'm not sure if this is due to running on Catalina or if I'm doing something wrong, but feel free to reject or just take what you need from this PR if it's at all useful.

Lastly - I experimented with passing in a URL like this as the track: "itmss://itunes.apple.com/us/station/listen-in-apple-music/st.xxxxxxx?cmd=AddStation" -- this points to my personal station, but got unreliable results. It would be great if there was a new configuration property to specify an iTunes URL location like above rather than a specific track name. This can be opened in the AppleScript with 'open location', which works when I hard code it into my local copy of the AppleScript.

ebaauw commented 4 years ago

I noticed that the state polling causes Music to reopen after you close it. So I added a check to determine if the Music app is running to the getState() and getSpeakerStates() functions. If it's not running some default JSON is returned to indicate the off state

Love this idea. Did you test this with multiple Airplay speakers? I'm not sure if only returning the Computer speaker won't cause issues. I'll test this weekend.

I also added a new getInitialState() function which is called by homebridge on startup. This will force Music to open which should allow for an accurate list of airplay speakers to be found regardless if Music was open. This is working for me locally.

Not sure I like an additional routine, getInitialState() in the script interface, but I appreciate the issue of the missing Airplay speakers. I think I would prefer an additional parameter to getState(), which would be ignored by the other scripts.
The structural solution would be to use the dynamic platform accessory model, so homebridge-music would remember previously exposed Airplay speakers. I think this would render getInitialState() (or the additional paramter to getState()) superfluous.

I tried compiling the scripts with the Makefile but am running into issues.

Yes, the scripts for iTunes and EyeTV need to be compiled on a macOS version before Catalina.

It would be great if there was a new configuration property to specify an iTunes URL location like above rather than a specific track name. This can be opened in the AppleScript with 'open location', which works when I hard code it into my local copy of the AppleScript.

Would it be possible to re-use the t parameter to setPlayerOn(), and if it's a URL call open location t instead of play track t?

spmcewen commented 4 years ago

Thanks for the feedback. I tested with a single airplay speaker and did not see any status errors in the Apple Home app despite the status does not being returned in the default JSON created if the app is closed.

I was thinking something similar about storing the previously found airplay speakers, but this is my first dive into both AppleScript and a homebridge plugin and was unsure how to proceed.

I did try passing in the URL as the track parameter in config.json. I was seeing unusual behavior where either playback would not work or the same song kept being replayed when started/stopped. I suspected it might have to do with this part in MusicPlatform.js

const track = obj.track || this.track ||

I'll play around with checking if track starts with 'itmss://' and doing open location instead of play track

spmcewen commented 4 years ago

I tried adding this to the Music.scpt:

if {t starts with "itmss://"} then
    open location t
else
    play track t
end if

The problem is that it will only work the first time. If playback is stopped and then turned back on again it restarts playing the last track that was played. Playback is no longer on the ittms:// URL.

ebaauw commented 4 years ago

I succeeded in adding a parameter initial to getState(), to merge the getInitialState() functionality. I now run getState(true) twice before creating the accessories, timeout seconds apart, to allow more time for Music/iTunes to discover the AirPlay speakers.

Also found some quirks in Music; it behaves slightly differently than iTunes. Will do the pre-Catalina scripts tomorrow.

I started refactoring the code, in preparation of moving to dynamic platform accessories. This is my oldest plugin and I haven't done any structural maintenance for years.

I still need to look at the location. I suppose MusicAccessory.prototype.setOn() should check if the track from config.json starts with itmss:// and pass that to the Applescript setPlayerOn(), instead of the current track. Or maybe add a config.json setting, to start playing always from that track.

ebaauw commented 4 years ago

OK, could you please try v0.2.16? I added a resetTrack setting to config.json, to reset the current track to the default track from config.json on stop. I don't have an Apple Music subscription, so I cannot test the itmss:// stuff, but it should work, with the Music script.

I haven't yet put the location logic in the Music-Airfoil script, nor in the iTunes and Airfoil scripts. Do you know if iTunes supports this?