Use Plex websocket payloads to reduce overhead#42332
Use Plex websocket payloads to reduce overhead#42332frenck merged 19 commits intohome-assistant:devfrom
Conversation
|
I think the session handling should be centralized more. Going to close this while I work on the refactoring. |
jjlawren
force-pushed
the
plex_use_websocket_payloads
branch
from
64286c3 to
88c4393
Compare
jjlawren
force-pushed
the
plex_use_websocket_payloads
branch
from
de1b27d to
40ec701
Compare
|
Coverage is low, but adding tests in the current framework would be overly complex. I'm reworking the tests to make this easier but that will need to be in a separate PR. |
| self._state = STATE_PAUSED | ||
| elif state == "stopped": | ||
| self.session = None | ||
| self.force_idle() |
There was a problem hiding this comment.
You are calling force_idle from a property setter, which then calls dispatcher_send. Please do not use a property setter if you have side effects.
There was a problem hiding this comment.
I was thinking dispatcher_send just sent an event to another listener instead of handling the scheduling itself. This can be moved out of the setter.
There was a problem hiding this comment.
Drop using X.setter. state is an inherited property and it is always confusing what is happening if other propertie are also updated because of it.
Process all sessions in server instance Only push state/position changes on most websocket updates Add convenience wrapper for session attributes
jjlawren
force-pushed
the
plex_use_websocket_payloads
branch
from
6528317 to
ae85cad
Compare
shocklateboy92
left a comment
shocklateboy92
left a comment
There was a problem hiding this comment.
This looks good overall to my (unfamiliar) eyes. I've heard python's type checking system is quite rudimentary compared to languages I'm used to, but I think having some sort of annotation will help with following the code (especially to a newcomer).
It also looks like you're not doing any throttling of state updates this time round. Is that because plex only sends updates via the websocket on a periodic basis?
| """Return representation of the session.""" | ||
| return f"<{self.session_key}:{self.sensor_title}>" | ||
|
|
||
| def update_media(self, media): |
There was a problem hiding this comment.
Adding types would really help with understanding this code for newcomers.
I believe media here is a plexapi.audio.Audio | plexapi.video.Video, right? (I don't know how to do union types in python, but that would be the syntax for typescript)
There was a problem hiding this comment.
I figured it out. The syntax is:
| def update_media(self, media): | |
| def update_media(self, media: Union[Movie, Episode, Clip, Track]): |
And of course, above you'd have to:
from plexapi.audio import Track
from plexapi.video import Clip, Episode, Movie
|
Agreed, some parts are difficult to follow and more hints would help. Throttling is mostly handled by the |
balloob
left a comment
balloob
left a comment
There was a problem hiding this comment.
One last comment. After that looks good.
5 participants
Proposed change
Plex websockets were previously only used to trigger updates from a central polling update method. Recently, #40773 started to pass the actual websocket payloads to the integration. This PR uses those payloads directly to avoid making unnecessary requests to the Plex server as most of the necessary information was already provided.
To make this possible, the active playback sessions are now maintained centrally with a dedicated class and pushed to the appropriate
media_playerandsensorentities as needed.Type of change
Additional information
Checklist
black --fast homeassistant tests)The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: