- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 197
Add plex connect provider #2279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add plex connect provider #2279
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new PlexConnect plugin to report playback timeline events back to Plex and refactors how Plex stream metadata is handled.
- Refactored get_stream_detailsandon_streamedto package Plex metadata into a structuredplex_datadict
- Introduced PlexTimelineReporterto send playback, pause, and stop events to Plex
- Added plugin manifest and wiring to load/unload the timeline reporter
Reviewed Changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 3 comments.
| File | Description | 
|---|---|
| music_assistant/providers/plex_connect/timeline.py | New PlexTimelineReporterclass for sending timeline updates | 
| music_assistant/providers/plex_connect/manifest.json | Plugin manifest for PlexConnect | 
| music_assistant/providers/plex_connect/init.py | Plugin provider setup/unload logic | 
| music_assistant/providers/plex/init.py | Updated Plex provider to emit structured plex_data | 
Comments suppressed due to low confidence (1)
music_assistant/providers/plex_connect/timeline.py:1
- Consider adding unit or integration tests for PlexTimelineReporterto cover the various state transitions and ensure timeline updates are sent correctly.
"""
| I think this should be split into smaller PRs again. The plugin should be its own? | 
| if not player.available or player.powered is False: | ||
| self.logger.info("Player %s is no longer available or powered off", pid) | ||
| pause_pos = int(data["elapsed_time"] * 1000) | ||
| self.logger.info("Arresto riproduzione per spegnimento player...") | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Italian?
| 
 I don't think since it's just the plugin and the other part in the Plex integration is required to make it work and generally it's a better implementation than before | 
| media_part: PlexMediaPart = media.parts[0] | ||
| audio_stream: PlexAudioStream = media_part.audioStreams()[0] | ||
|  | ||
| plex_data = { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One possible small enhancement is to specify the dataformat with a TypedDict so you have a bit of help in the editor what to expect for the fields.
| Please fix the last remaining mypy typing issues and then this PR is good to go | 
| I ran mypy and all the issues are not from the plex files so I'm not sure what to fix | 
| # Config Entries are used to configure the Provider if needed. | ||
| # See the models of ConfigEntry and ConfigValueType for more information what is supported. | ||
| # The ConfigEntry is a dataclass that represents a single configuration entry. | ||
| # The ConfigValueType is an Enum that represents the type of value that | ||
| # can be stored in a ConfigEntry. | ||
| # If your provider does not need any configuration, you can return an empty tuple. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # Config Entries are used to configure the Provider if needed. | |
| # See the models of ConfigEntry and ConfigValueType for more information what is supported. | |
| # The ConfigEntry is a dataclass that represents a single configuration entry. | |
| # The ConfigValueType is an Enum that represents the type of value that | |
| # can be stored in a ConfigEntry. | |
| # If your provider does not need any configuration, you can return an empty tuple. | 
| Plex content playback. | ||
| """ | ||
| self.logger.debug("Setting up Plex timeline reporter...") | ||
| self._session = aiohttp.ClientSession() | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the existing global clientsession instead of creating your own:
mass.http_session or mass.http_session_no_ssl
| if self._session: | ||
| await self._session.close() | ||
| self._session = None | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if self._session: | |
| await self._session.close() | |
| self._session = None | 
not needed when global clientsession is used
| def _validate_plex_data(self, plex_data: dict) -> bool: | ||
| required_keys = ["rating_key", "server_url", "token", "machine_identifier"] | ||
| if not all(k in plex_data for k in required_keys): | ||
| missing = ", ".join(k for k in required_keys if k not in plex_data) | ||
| self.logger.debug("Missing required Plex data in streamdetails: %s", missing) | ||
| return False | ||
| return True | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this seems to hard depend on the plex provider itself to send this data ?
Maybe then it makes sense to add "depends_on" in the manifest and set it to plex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes without the Plex integration it doesn't work. Does the dependency need to be added in the manifest.json for the provider?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manifest
| Marked as draft, awaiting the review comments to be addressed. | 
| @chicco-carone Looking forward to this one. This may be outside the scope of this PR, but can we also sync ratings between Plex and MA? | 
| Superseded by #2512 Sorry for having to abandon your work here @chicco-carone but the state was stale for a while and a simpler approach came in which is now already merged. Feel free to improve on top of that later. | 
This pull request introduces enhancements to the Plex integration in Music Assistant, focusing on improving stream data handling and adding a new plugin provider for reporting playback timeline events. The most important changes include restructuring stream data, updating playback tracking logic, and introducing the
PlexConnectplugin provider.Enhancements to Plex Stream Data Handling:
get_stream_detailsmethod to use a structured dictionary (plex_data) for stream metadata, replacing the direct use ofplex_trackin thedatafield ofStreamDetails. This improves clarity and maintainability and is used by the plex connect plugin to report back to the server the data.on_streamedmethod to use the newplex_datastructure for marking tracks as played, replacing direct attribute access with dictionary keys.Introduction of the
PlexConnectPlugin Provider:PlexConnectplugin provider to monitor and report Plex playback timeline events. This plugin adds support to synchronize track playback back to the plex server.manifest.jsonfile for thePlexConnectplugin, specifying its metadata, requirements, and documentation link.This pr contains the remain of the code from #2228