svrooij / node-sonos-ts

:speaker: Sonos control library, use this library in your own appliction.
https://sonos-ts.svrooij.io/
MIT License
84 stars 18 forks source link

Update metadata for streams even if URI doesn't change #31

Closed cheanrod closed 4 years ago

cheanrod commented 4 years ago

I removed the check for changed track URI. The check prevented the update of track meta data for radio streams. When a stream is played the track URI doesn't change but the track meta is constantly updated.

As a side effect, the meta data events are now published periodically even if the meta data didn't change. So an open question is: Is there a way to identify if the meta data changed from one event to another?

svrooij commented 4 years ago

I like the fact you would be the first external contributor! And I do agree with your observation, but you also removed the change check on the uri, did that have a specific reason? Maybe it’s better to keep the check on the uri changing and only emit the new metadata.

That way the logic doesn’t change for the current track uri. I myself got a lot of logic depending om the uri changes (and I never listen to streams, so haven’t encountered the metadata not changing)

And to answer your question I didn’t know the metadata could change without the uri changing, I think the o ly way you would be able to tell if the metadata changed is be to save it in the SonosDevice and checking when you get a new version. (To compare 2 JavaScript objects, I would suggest converting them to a JSON string and do a string compare, but that seems like overkill, so just emitting them if received isn’t a problem to me)

cheanrod commented 4 years ago

I added a commit that adapts the PR as you suggested. Only emiting the new meta data works fine for me.

And I agree that implementing new logic that adds caching of meta data is overkill for me too. If it were already there we could use it. But the update rate of events in my tests don't make it necessary for me.

svrooij commented 4 years ago

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

The release is available on:

Your semantic-release bot :package::rocket:

svrooij commented 4 years ago

@cheanrod because you followed the semantic release commit message, your first contribution is automatically released! Thanks for this contribution!

If you have other stuff you would like to implement you can also create an issue for it, to keep track of a list of things still to do. And if you build a cool app using this library you should also add it to https://github.com/svrooij/node-sonos-ts#others-using-node-sonos-ts