Refactor reproduce_state for scene component

[MartinHjelmare]

Description:

• Add tests to reach full coverage for helpers/state.py.
• Refactor reproduce_state function in helpers/state.py. Add two dicts,
  service_attributes and service_to_state. Use these in combination
  with the dict of services per domain from ServiceRegistry, to find
  the correct service to use in a scene state change.
• Add ATTR_CODE and ATTR_CODE_FORMAT in const. Import these in
  alarm_control_panel and lock platforms instead of making duplicate
  constants in multiple modules.
• Use ATTR_MEDIA_CONTENT_TYPE and ATTR_MEDIA_CONTENT_ID in media_player
  platform in SERVICE_PLAY_MEDIA and play_media methods, instead of
  'media_type' and 'media_id'.
• Fix PEP257 in modified files.

Related issue (if applicable):
Fixes #1166
Example entry for configuration.yaml (if applicable):
Nothing new, but we should update the docs with more examples.

scene:
  - name: Romantic
    entities:
      light.tv_back_light: on
      light.ceiling:
        state: on
        xy_color: [0.33, 0.66]
        brightness: 200

  - name: Movies
    entities:
      light.tv_back_light:
        state: on
        brightness: 100
      light.ceiling: off
  - name: play_batman
    entities:
      media_player.universal:
        media_content_type: 'movie'
        media_content_id: 'batman'

Checklist:

• Local tests with tox run successfully.
• TravisCI does not fail. Your PR cannot be merged unless CI is green!
• Fork is up to date and was rebased on the dev branch before creating the PR.
• Commits have been squashed.
• If the code does not interact with devices:
  • Tests have been added to verify that the new code works.

[MartinHjelmare]

Should I move the dicts out to the module level and make them global constants instead?

[balloob]

Yeah, they are not dynamic so no need to define in-line.

[MartinHjelmare]

All services that I could find and that I thought made sense to activate as a result of a scene state update are now supported.

Services that were not added were, among others:
media_player services with no corresponding state or specific attribute, eg next track
mqtt publish and mqtt publish_template
device_tracker see service
toogle service

One thing that would be nice to add is a way of telling the rollershutter which position to go to. But I couldn't figure out an easy way of solving that. Something to think about...

[MartinHjelmare]

@balloob How would you feel about adding two attributes to every service. One attribute would be the state which the service activates for an entity, the other a list of required attributes in data to call the service.

With such a change the two dicts I created for reproduce state would not need to be maintained.

[balloob]

Services already register which attributes they take in. See for example light/services.yaml. I think it's a great idea to add something like result_state to that file.

This data is registered with the service registry (hass.services). It is currently only exposed calling .services but that translates all services to a dict. I think that it should instead of translating things to a dict actually return the _services dictionary. Translating to a dict should happen in api.py (I think that's the only place it is used right now).

[balloob]

This is a very nice clean up ! Once you address both your own comments I think it's good to go 🐬

I would suggest we merge this before you migrate reproduce_state to use service description data.

[MartinHjelmare]

Yeah, I saw services.yaml. My problem with those is that we have to maintain code at two places for every service, instead of configuring all attributes of the service when it's registered. This would be more natural and less error prone, in my opinion.

At the same time, I can see that it's potentially a lot of attributes per service, so it's nice to keep that code mass separate. Would it be possible to, somehow, make services.yaml required and maybe add even more functionality there, and have no configure possibilities when calling to register a service?

Correct me if I've misunderstood how services.yaml and registering a service is connected.

[balloob]

services.yaml is passed in when a service is registered. Because it's
metadata I decided to move it outside of the main component file.

On Sun, Mar 6, 2016 at 10:23 AM, Martin Hjelmare notifications@github.com
wrote:

Yeah, I saw services.yaml. My problem with those is that we have to
maintain code at two places for every service, instead of configuring all
attributes of the service when it's registered. This would be more natural
and less error prone, in my opinion.

At the same time, I can see that it's potentially a lot of attributes per
service, so it's nice to keep that code mass separate. Would it be possible
to, somehow, make services.yaml required and maybe add even more
functionality there, and have no configure possibilities when calling to
register a service?

Correct me if I've misunderstood how services.yaml and registering a
service is connected.

—
Reply to this email directly or view it on GitHub
#1485 (comment)
.

PaulusSchoutsen.nl
It's nice to be important but it's more important to be nice.

[balloob]

I don't think it should not be required. If a service is not registering
anything, it just defaults to empty everything.

On Sun, Mar 6, 2016 at 10:27 AM, Paulus Schoutsen <paulus@paulusschoutsen.nl

wrote:

services.yaml is passed in when a service is registered. Because it's
metadata I decided to move it outside of the main component file.

On Sun, Mar 6, 2016 at 10:23 AM, Martin Hjelmare <notifications@github.com

wrote:

Yeah, I saw services.yaml. My problem with those is that we have to
maintain code at two places for every service, instead of configuring all
attributes of the service when it's registered. This would be more natural
and less error prone, in my opinion.

At the same time, I can see that it's potentially a lot of attributes per
service, so it's nice to keep that code mass separate. Would it be possible
to, somehow, make services.yaml required and maybe add even more
functionality there, and have no configure possibilities when calling to
register a service?

Correct me if I've misunderstood how services.yaml and registering a
service is connected.

—
Reply to this email directly or view it on GitHub
#1485 (comment)
.

PaulusSchoutsen.nl
It's nice to be important but it's more important to be nice.

PaulusSchoutsen.nl
It's nice to be important but it's more important to be nice.

[MartinHjelmare]

I guess my main issue is that today you can register a service that requires attributes for excecution, but it's optional to describe these attributes in services.yaml. This has lead to most services.yaml being empty, and the only way of finding the attributes that are required for a service is by manually reading through the code. Same problem with target state and services.

[balloob]

services.yaml was meant to be the documentation. It is also made available in the frontend when calling a service. They are indeed not all filled in but that's the problem we should try to solve.

[MartinHjelmare]

Yes, agree.

[MartinHjelmare]

Should be good now.

I'll have to look more closely at the service registry and the translations you were talking about. I'm not totally in on what you meant, but I think I'll figure it out, once I read the source.

"This data is registered with the service registry (hass.services). It is currently only exposed calling .services but that translates all services to a dict. I think that it should instead of translating things to a dict actually return the _services dictionary. Translating to a dict should happen in api.py (I think that's the only place it is used right now)."