Register 'firetv.adb_command' service

[JeffLIrion]

Description:

Register media_player.firetv_adb_cmd service so that users can send custom ADB commands to their device(s).

Related issue (if applicable): fixes #

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#8722

Checklist:

• The code change is tested and works locally.
• Local tests pass with tox. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated in home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• New dependencies have been added to the REQUIREMENTS variable (example).
• New dependencies are only imported inside functions that use them (example).
• New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
• New files were added to .coveragerc.

If the code does not interact with devices:

• Tests have been added to verify that the new code works.

[elupus]

should custom services really be in media_player?

[MartinHjelmare]

Until we've moved the platform to its own component, it's ok. Moving all platforms under component packages is a work in progress. New platforms should go under their own components and register platform specific services under their component.

[andrewsayre]

Until we've moved the platform to its own component, it's ok. Moving all platforms under component packages is a work in progress. New platforms should go under their own components and register platform specific services under their component.

We're enforcing this rule now. This should be moved to a new component to register the service. One less breaking change if done later.

[MartinHjelmare]

It's only enforced for new platforms.

That's at least my understanding from reading the messages. I wouldn't mind enforcing it further.

[JeffLIrion]

Until we've moved the platform to its own component, it's ok. Moving all platforms under component packages is a work in progress. New platforms should go under their own components and register platform specific services under their component.

We're enforcing this rule now. This should be moved to a new component to register the service. One less breaking change if done later.

I'm not very familiar with the planned architectural changes. However, seeing as this migration hasn't happened yet and there are plenty of component-specific services under the media_player platform, I would prefer to get this service integrated first and move the Fire TV component (including this service) into its own platform when all the other components get moved.

[JeffLIrion]

@MartinHjelmare, @elupus, @andrewsayre what should I do for this pull request?

[MartinHjelmare]

I think it's a good point that we should avoid breaking changes if we can.

• Make a firetv package with an init module, __init__.py, with only docstring.
• Git move the firetv.py media_player platform to that firetv package and name it media_player.py.
• Register the service under the new firetv domain, from the media_player platform.

[JeffLIrion]

I think it's a good point that we should avoid breaking changes if we can.

• Make a firetv package with an init module, __init__.py, with only docstring.
• Git move the firetv.py media_player platform to that firetv package and name it media_player.py.
• Register the service under the new firetv domain, from the media_player platform.

Thanks.

Edit: I think I figured out the answers to the first two questions.

1. Will this change the configuration for users? Or will it still be:

   media_player:
   - platform: firetv
     name: Fire TV
     ...

   • Answer: no change
2. Will services such as media_player.media_play become firetv.media_play?
   • Answer: no
3. Am I supposed to define a constant DOMAIN = 'firetv'? Should this be in __init__.py or media_player.py?

[JeffLIrion]

Simple bash script for moving firetv to its own platform:

#!/bin/bash

# running in: home-assistant/homeassistant/components/media_player
export PLATFORM="firetv"

mkdir -p ../$PLATFORM
if [ ! -f "../$PLATFORM/__init__.py" ]; then
    # TODO: copy the docstring
    touch ../$PLATFORM/__init__.py;
fi

git add ../$PLATFORM/__init__.py
git mv $PLATFORM.py ../$PLATFORM/media_player.py

[MartinHjelmare]

1. Configuration will remain the same.
2. No, standard services will remain registered under media_player domain. Only the new custom service should be registered under the firetv domain.
3. It's not required to define a domain constant in the firetv media_player module. If you want to use a constant when registering the service you can eg define FIRETV_DOMAIN = 'firetv' in that module.

It's not clear to me what domain we consider the moved firetv media_player platform to belong to. If it's media_player or firetv?

[JeffLIrion]

@MartinHjelmare I called it DOMAIN, but I can change it to FIRETV_DOMAIN.

Also, I don't know why the build is failing. I don't think it's due to any changes I made in this pull request.

[JeffLIrion]

Also, in the services tab, the "Entity" menu doesn't populate correctly. Any idea what I need to do to fix that?

[JeffLIrion]

The build failed, but I'm pretty sure it has nothing to do with my changes.

lint runtests: commands[0] | python script/gen_requirements_all.py validate
******* ERROR
requirements_all.txt is not up to date
Please run script/gen_requirements_all.py
ERROR: InvocationError for command '/home/travis/build/home-assistant/home-assistant/.tox/lint/bin/python script/gen_requirements_all.py validate' (exited with code 1)

[MartinHjelmare]

We have moved the file that contains a requirement spec. Please follow the instructions and run the script.

Also update coveragerc.

[JeffLIrion]

I looked in the logs for both of the failed tests and it doesn't look like the failures have anything to do with the Fire TV platform.

[balloob]

I agree with registering services under the name of the integration . That is what we should do moving forward.

[JeffLIrion]

The build passed! Any other changes needed before merging this?

[MartinHjelmare]

Nice!

[JeffLIrion]

Thanks @MartinHjelmare for your help with this PR and the previous one, and thanks to @carstenschroeder for inspiring this change with #20673.