supported_features Vs current_supported_features about architecture

[lucasweb78]

It looks like we've reached a consensus on this and are moving the media_player platforms to using a static set of supported features (only plex is left cc @ryanm101). Any objections to closing this discussion?

[lucasweb78]

Adding current_supported_features is fairly straight forward:

• It could be added to Entity and default to supported_features
• supported_features can be updated in components that currently make it dynamic to be static e.g. apple_tv
• current_supported_features can be updated in components to expose current features dynamically e.g. apple_tv
• The media player ui component can be updated to use current_supported_features
• Any other components that rely on dynamic supported_features can also be updated

[balloob]

I wouldn't say it was a proposal from my part, it was merely an idea as part of a brainstorm.

I think that supported_features should explain the capabilities of a device. This can be used as a guide to which services can be used with which parameters. A write up of how all of these features map to services/parameters should be added to https://dev-docs.home-assistant.io

Things that are supported_feature "worthy":

• Light that supports color (should it be type of color?)
• Light that supports brightness

Sometimes however, we see that we don't need a supported_features flag because if something is supported can be derived from other things that are available in the state (ie if a climate entity has no swing_list, one cannot set the swing mode).

[lucasweb78]

I agree that supported_features should explain the capabilities of a device.

I'm not sure I fully understand how this would work with also being able to derive features that are available in the state. Does this mean you would need to check both the supported_features and state to determine a devices capabilities, or would supported_features be calculated based on information on the state? Ideally we just want a single property that can be read to determine everything a device supports.

The other question is do we need a current_supported_features or active_features which specifies what a device supports in it's current state e.g. an idle media player does not support play/pause so the UI component should not show the play/pause button?

Updating the documentation makes sense and would be happy to help with that. I would also like to figure out if we can start moving components to using supported_features to describe all of the devices capabilities and what impact this has on components that expose supported_features based on current state e.g. Apple TV Media Player

[balloob]

I've thought about it a bit more and we should go for the first implementation: supported_features should be all of the entities supported features. We should not have our entity design be steered by the frontend.

For "active supported features" I want to make sure that we do not use the word "features" or the current flags as I don't want to conflate concerns. Maybe call it something as "player functionality" or something. It can still work with bit flags like supported features, just should be clearly differently called.

[lucasweb78]

Sounds good to me. How do you normally manage a somewhat breaking change like this? I would like to start moving entities to ensuring supported_features list all capabilities especially given the impact on Alexia discovery.

I’m happy to work through the media players as I already have a PR for Apple TV and understand the impact this change has on the UI component (its mostly around hiding showing controls).

The options are:

• add the new concept of active_player_functionality or similar and update the UI card to hide/show controls based on this first and then update the supported_features

• update supported_features first and live with the media player card not hiding showing controls based on state (which is already the case for most of the media player implementations) and then implement the UI changes later

Similar decisions also need to be made for other entities that have cards which rely on dynamic supported_features to decide when to hide/show elements based on the entities current state

[balloob]

I think that only the media player has the frontend depend on supported_features being dynamic.

To make the transition easier, I would expect us to indeed start with adding the new functionality attribute to the media player. We will have to look at the frontend to see what kind of checks we actually need.

Source code for media player is here

Some things can still be derived from supported_features, like turn on/turn off. I think that we just need prev track, next track, pause/play/stop (not completely sure what is going on here)

[lucasweb78]

The UI changes look fairly straight forward:

• media-player-model.html Lines 71 - 107

• more-info-media_player.html Lines 254 - 264

What is the difference between state-card-media_player.html and ha-media_player.html?

In HA it is a case of:

• adding active_player_functionality (or other name) here
• adding the new flags
• Implementing support for active_player_functionality in the different media components

Questions:

1. While I agree we should use new flags to avoid confusion, can we use the same values for the flags for the bitwise checks or do we need new values?

2. Do we need to update all media components or is there a sensible default we can use in media_player/init.py?

[balloob]

1. New values. Let's make it very clear that it's different. (also we start at 2^0 anyway)

2. I guess update. Although I don't mind to not do that all in 1 PR.

[lucasweb78]

I've started mapping media player support_features to services and am going to update the docs as a first step. I just got bitten by not understanding which services map to which supported_features and have had to submit another PR for the Alexa StepSpeaker to StepVolume integration.

I also start to define active_player_functionality. Am still a little unsure on how the bitwise values are assigned but will take a best guess and get a WIP out for feedback

[amelchio]

This seems like much ado about (almost) nothing. Can we just remove the dynamic features?

It is crazy to have each individual platform add/remove a bunch of SUPPORT_PLAY-like bits based on their current playing state. That will give a lot of corner cases with inconsistencies and bugs.

Rather than a generic framework, I think much of this could be solved by adding e.g. a first_track: True attribute when the Previous control should be disabled.

For now, though, I propose that we just ignore the issue and have a visible no-op Previous control in that situation. My DVD remote control always worked like that.

According to a survey by @lucasweb78, only three media players use dynamic controls so it will be a simple cleanup.

[lucasweb78]

I've been thinking about this some more and tend to agree. I'm still working on improving the documentation for the media_player component so that it's clear what flag maps to which service but I am not sure about introducing a second set of flags just for managing the UI controllers.

It would be good to figure out a path forward as right now the Apple Media Player still doesn't have all of it's capabilities correctly discovered by Alexa (unless you run my PR home-assistant/core#12167 as a custom_component).

I've been running the above PR for a few weeks and not having the media_player controls in the UI disappear when it's not in use has not caused any problems and is actually, IMO, nicer as the UI component doesn't keep shifting in size and changing the layout.

[balloob]

👍 for making supported_features static across Home Assistant. It follows very much the "backend should not care nor try to solve frontend issues".

[ryanm101]

@lucasweb78 What needs done for plex?

[emontnemery]

The conclusions in this issue have been reconsidered in #985, we now allow updating supported_features.