Basic AudioScrobbler plugin #1850
Conversation
| if self._network is None: | ||
| return | ||
|
|
||
| item = await self.mass.music.get_item_by_uri(event.object_id) |
There was a problem hiding this comment.
event.data also contains a media_item attribute so you wont have to fetch it , saves a potential api call
There was a problem hiding this comment.
doh, no idea how I missed that, I'll update that, thanks!
There was a problem hiding this comment.
I figured out why I missed that. Because it's not true :P data["media_item'] contains the reference as well, not the item. I played around a bit with passing the actual item, but I got serialization errors.
There was a problem hiding this comment.
I updated the object with the info present. We must prevent fetching full item details where/if we can to save api calls
|
Out of interest will you be able to scrobble to last.fm and libre.fm at the same time? Have two instances for example? |
| self.logger.exception(err) | ||
|
|
||
| async def _on_mass_media_item_played(self, event: MassEvent) -> None: | ||
| """Media item has finished playing, we'll scrobble the track.""" |
There was a problem hiding this comment.
I believe the Subsonic provider scrobbles if half the track is played. Is there a standard for this? (Reference: music-assistant/support#3203 (comment) )
There was a problem hiding this comment.
It's one of the things I want to look at, 50% is fairly common, but often configurable.
For now I just focussed on the generic config and basic events, as currently it's not super easy to trigger something based on percentage played.
There was a problem hiding this comment.
_on_mass_media_item_played will always be triggered and includes the amount of seconds played.
So you can make your own logic together with the item's duration
Check the event.data of this event, it has all the info you need.
There was a problem hiding this comment.
I've added a basic check for now, but usually the approach is a bit different. For example, often you'd actually detect length that's being played, so skipping ahead would not cause a scrobble.
And with the current setup, when a song gets paused for example, played and paused near the end of the song, it gets scrobbled every time. I could try to detect that, but having that work while also accounting for songs that are on repeat would be annoying.
These are all nice to haves though, we can iterate later if we want.
|
|
||
| queue: PlayerQueue = event.data | ||
| if queue.state == PlayerState.PLAYING and queue.current_item.media_type == MediaType.TRACK: | ||
| track: Track = queue.current_item.media_item |
There was a problem hiding this comment.
We have a helper called is_track which implements a typing TypeGuard. If you use it here you won't need the explict type.
There was a problem hiding this comment.
I've used it in 2 places, but honestly the typing support in Python confuses me a bit and I probably don't have my editor setup correct to understand the TypeGuard, so I still left the : Track typehint here.
Not at the moment and honestly I've no idea what it would involve. I saw there is an option to allow multiple instances of a plugin, so maybe that would be enough, but I would need to some rigorous testing first. |
| self._network.scrobble( | ||
| artist=track.artist_str, | ||
| title=track.name, | ||
| timestamp=time.time(), | ||
| mbid=track.mbid, | ||
| album=track.album.name if track.album else None, | ||
| ) |
There was a problem hiding this comment.
this is doing a blocking call - you need to either wrap the calls to this library into an executor or do the calls yourself using an async web client.
There was a problem hiding this comment.
Done, it's been a while since I worked in an eventlooped system. Combined with my lack of python experience I just saw async and assumed all was good 😂
- Moved http calls off the main loop - Moved init to setup - Avoid duplicates - Generic exception handling - Ignore songs that are played less than 50%
| def wrap(cb: Callable[[MassEvent], None]): | ||
| async def handler(event: MassEvent) -> None: | ||
| try: | ||
| await cb(event) | ||
| except Exception as e: | ||
| self.logger.exception(e) | ||
|
|
||
| return handler | ||
|
|
||
| # subscribe to internal events | ||
| self.mass.subscribe(wrap(self._on_mass_queue_updated), EventType.QUEUE_UPDATED) | ||
| self.mass.subscribe(wrap(self._on_mass_media_item_played), EventType.MEDIA_ITEM_PLAYED) |
There was a problem hiding this comment.
I'm not a fan of this wrapping of event logic tbh
The core is already creating tasks for each event handler.
Please just subscribe a callback as intended
There was a problem hiding this comment.
Ah yeah, I forgot to add a comment about it. I'm not a fan either, so I'm happy to remove it.
I added it because I had some trouble with silent exceptions, which was very frustrating.
There was a problem hiding this comment.
hmmm, exceptions should never go silent as we have a global catch-all exception handler for uncaught ones.
I'll have to look into that, maybe a bug.
There was a problem hiding this comment.
I don't know if there's a different in Python, but among others I ran into method not found kind of exceptions that didn't get logged. I imported is_track within the TYPE_CHECKING block, which (obviously) didn't work.
There was a problem hiding this comment.
these should be catched by the catch-all exception handler. Are you running the debugger in vscode according to the predefined settings?
|
@wjzijderveld once you get close to merging can you provide some text for the docs? We have the following headings. You don’t need text for all of them. Please see the other providers for ideas of what to add. Just the text is fine I will do the actual PR for the docs as a number of things need to be done. FEATURES |
An initial basic implementation to support scrobbling via Last.fm and Libre.fm (I've not tried it with Libre yet).
It probably could use some better explanation how to create an key and secret.
Also this is my first time hacking on music-assistant and one of the few times I worked in Python, so please let me know if I'm being super dumb somewhere 😅