Add media position properties#10076
Conversation
I don't get why pylint is thinking that these functions are unavailable. They are defined here:
EDIT: Here is a snippet that essentially does the same as |
|
You're using Python 3.5+ |
|
Thanks for the hint. What I don't get is why pylint isn't complaining about the |
|
That certainly is strange, but that's what I'd try first. |
|
You were right, @armills |
emlove
left a comment
There was a problem hiding this comment.
A few small notes, but the biggest issue is the remaining time / last updated need to not change on every update.
| self._current_program = None | ||
| self._media_duration = None | ||
| self._media_remaining_time = None | ||
| self._media_ = None |
There was a problem hiding this comment.
Looks like a copy-paste artifact got left in here.
| # Media progress info | ||
| self._media_duration = \ | ||
| pyteleloisirs.get_program_duration(program) | ||
| self._media_remaining_time = \ |
There was a problem hiding this comment.
Media position and media remaining time need to only change when the player actually changes state. See vlc for an example https://github.com/home-assistant/home-assistant/blob/08b0629eca6417a2bb1b6f3c805ff19208484e4d/homeassistant/components/media_player/vlc.py#L70-L72
There was a problem hiding this comment.
Okay, but the media duration only depends on the current time. So this is pretty much guaranteed to be updated every time.
| @asyncio.coroutine | ||
| def async_update(self): | ||
| """Retrieve the latest data.""" | ||
| import pyteleloisirs |
There was a problem hiding this comment.
We should either expose get_program_duration and get_remaining_time off of liveboxplaytv, or specifically call out pyteleloisirs in the requirements list since we're using it directly. We shouldn't implicitly depend on liveboxplaytv to pull in the requirement if we're calling it directly.
|
The rest of this looks good, but could you elaborate on #10076 (comment) ? I don't understand why the media duration would be changing constantly. We don't want to introduce changes that will create constant state updates. |
|
In |
|
Hmm, looking at the VLC example, I don't think that one is doing what we thought either. Going to ping @balloob for a weigh-in. |
|
We should never ever add the current position in the state. The frontend will calculate this when needed by doing |
|
If VLC does this, we should remove it from there as well. |
|
So it means that current position (and updated at) should only be updated when the state of the player changes. So when it goes from play to pause etc. |
|
okay @balloob you are saying it should be something along the lines of: @asyncio.coroutine
def async_update(self):
state = self.refresh_state()
if state != self._state: # state changed
self._state = state
self._media_duration = XXX
self._media_remaining_time = XXXBut this won't update the current program duration ( |
|
I don't know how often liveboxplaytv or vlc send updates. It's not really a problem if they only send updates at state changes. If it's sent too often then something like this should be ok (untested): Or maybe it should compare with something larger than 1. I don't know if pyteleloisirs only returns whole seconds and how big latencies may be. Writing this I realize that it should be elapsed_time (or position), not remaining_time. |
|
Please test this |
…into dev * 'dev' of https://github.com/home-assistant/home-assistant: Disable html5 notify dependency (home-assistant#11135) ISY994 sensor improvements (home-assistant#10805) Allow using more than one keyboard remote (home-assistant#11061) set default utc offset to 0 (home-assistant#11114) Add problem device class (home-assistant#11130) Always consume the no_throttle keyword argument. (home-assistant#11126) Skip HASS emulated Hue bridges from detection. (home-assistant#11128) update pyripple (home-assistant#11122) Add media position properties (home-assistant#10076) Fixed typo in automation.py (home-assistant#11116)
Description:
This adds progress information to
media_player.liveboxplaytv.