New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable native support + ADB authentication for Fire TV #17767

Merged
merged 36 commits into from Nov 19, 2018

Conversation

Projects
None yet
7 participants
@JeffLIrion
Contributor

JeffLIrion commented Oct 24, 2018

Description:

Related issue (if applicable): fixes #13482

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

Forum thread: https://community.home-assistant.io/t/native-support-for-fire-tv/64448

Example entry for configuration.yaml (if applicable):

# Example configuration.yaml entry
media_player:
  # a device that does not require ADB authentication
  - platform: firetv
    name: Fire TV 1
    host: 192.168.0.111

  # a device that does require ADB authentication
  - platform: firetv
    name: Fire TV 2
    host: 192.168.0.222
    adbkey: "/config/android/adbkey"

  # a device for which getting the current app (source) and the running apps (sources) cause issues
  - platform: firetv
    name: Fire TV 3
    host: 192.168.0.123
    get_source: false
    get_sources: false

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

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

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

Breaking Changes (short version)

The host and port configuration variables now refer to the Fire TV device itself, whereas before they referred to the firetv server running in a Python 2 environment.

Breaking Changes (longer version)

Previously, the Fire TV component required the user to run a firetv server in a Python 2 environment, and Home Assistant would poll that server for information about the Fire TV device(s). In this setup, the host and port configuration variables referred to the firetv server.

Recent updates to the firetv and adb libraries have enabled the firetv package to run in a Python 3 environment. In this update, Fire TV devices are monitored and controlled directly by Home Assistant itself -- no third party Python 2 firetv server required! Accordingly, the host and port configuration variables now refer to the Fire TV device itself.

Furthermore, this update provides support for ADB authentication, which nearly all Fire TV devices now require. To enable ADB authentication, the user must follow the instructions in home-assistant/home-assistant.io#7062 and add the adbkey configuration variable.

@dshokouhi dshokouhi referenced this pull request Oct 24, 2018

Closed

WIP: Add new platform AndroidTv/Android devices #16975

4 of 8 tasks complete
@JeffLIrion

This comment has been minimized.

Contributor

JeffLIrion commented Oct 24, 2018

I believe this needs a "breaking-change" label, since the user's configuration will need to be updated.

@rytilahti

Thanks for the PR, this is a very welcome effort! I did a brief testing and the basics (changing apps, playing & pausing) seemed to be working at first, but suddenly it stopped updating the play/pause state. Could be that me trying to connect with another adb instance to the same device got it confused (stays in 'playing' even when being paused, in both netflix and amazon videos).

Show resolved Hide resolved homeassistant/components/media_player/firetv.py Outdated
Show resolved Hide resolved homeassistant/components/media_player/firetv.py Outdated
@JeffLIrion

This comment has been minimized.

Contributor

JeffLIrion commented Oct 26, 2018

So I guess this PR is just waiting for packages to be published on pypi. Aside from that, are there any other changes that need to be made?

JeffLIrion added some commits Oct 26, 2018

@houndci-bot

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'
@houndci-bot

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'
@JeffLIrion

This comment has been minimized.

Contributor

JeffLIrion commented Oct 29, 2018

OK, so the lint and pylint tests aren't passing. Here is what I saw in the test logs that looks important. Advice on how to pass these tests would be appreciated.

lint

******* 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)

When I ran python script/gen_requirements_all.py on my machine it made a bunch of changes to the requirements_all.txt and requirements_test.txt files, but it didn't add anything about the adb-homeassistant or firetv packages. I'm not sure what the problem is here...

pylint

************* Module homeassistant.components.media_player.firetv
homeassistant/components/media_player/firetv.py:64:11: W0212: Access to a protected member _adb of a client class (protected-access)
homeassistant/components/media_player/firetv.py:64:11: W0212: Access to a protected member _firetv of a client class (protected-access)
homeassistant/components/media_player/firetv.py:115:8: E0611: No name 'FireTV' in module 'firetv' (no-name-in-module)
homeassistant/components/media_player/firetv.py:224:8: W0702: No exception type(s) specified (bare-except)
homeassistant/components/media_player/firetv.py:162:19: W0212: Access to a protected member _adb of a client class (protected-access)
homeassistant/components/media_player/firetv.py:171:21: W0212: Access to a protected member _screen_on of a client class (protected-access)
homeassistant/components/media_player/firetv.py:177:21: W0212: Access to a protected member _awake of a client class (protected-access)
homeassistant/components/media_player/firetv.py:209:25: W0212: Access to a protected member _wake_lock of a client class (protected-access)
homeassistant/components/media_player/firetv.py:217:21: W0212: Access to a protected member _wake_lock of a client class (protected-access)
homeassistant/components/media_player/firetv.py:225:12: W1201: Specify string format arguments as logging function parameters (logging-not-lazy)
homeassistant/components/media_player/firetv.py:227:12: W0212: Access to a protected member _adb of a client class (protected-access)
------------------------------------
Your code has been rated at 10.00/10

The errors are:

  • W0212: Access to a protected member _adb of a client class (protected-access)
    • I could have pylint ignore this error
  • E0611: No name 'FireTV' in module 'firetv' (no-name-in-module)
    • It thinks it's importing itself...
  • W0702: No exception type(s) specified (bare-except)
    • I did this as a catch-all
  • W1201: Specify string format arguments as logging function parameters (logging-not-lazy)
    • I think that's a false positive
@JeffLIrion

This comment has been minimized.

Contributor

JeffLIrion commented Oct 29, 2018

@rytilahti, please let me know what you'd like me to change so that I can get this PR approved.

JeffLIrion added some commits Oct 31, 2018

@rytilahti

This comment has been minimized.

Contributor

rytilahti commented Oct 31, 2018

Looking at the requirements.txt diff, there are lots of unwanted changes there, so that's why it is complaining.

Show resolved Hide resolved homeassistant/components/media_player/firetv.py Outdated
Show resolved Hide resolved homeassistant/components/media_player/firetv.py Outdated
Show resolved Hide resolved homeassistant/components/media_player/firetv.py Outdated
Show resolved Hide resolved homeassistant/components/media_player/firetv.py Outdated
Show resolved Hide resolved homeassistant/components/media_player/firetv.py
Show resolved Hide resolved homeassistant/components/media_player/firetv.py Outdated
Show resolved Hide resolved homeassistant/components/media_player/firetv.py Outdated
Show resolved Hide resolved homeassistant/components/media_player/firetv.py Outdated
Show resolved Hide resolved homeassistant/components/media_player/firetv.py Outdated

JeffLIrion added some commits Oct 31, 2018

@JeffLIrion

This comment has been minimized.

Contributor

JeffLIrion commented Nov 13, 2018

If firetv 1.0.7 becomes available on pypi, the build will pass. Would you then be willing to merge this PR?

@JeffLIrion

This comment has been minimized.

Contributor

JeffLIrion commented Nov 15, 2018

I switched from using a boolean to using threading.Lock(). If everything looks good now, then I'll request that my PR for firetv 1.0.7 gets merged. Does anything else need to be changed?

@JeffLIrion

This comment has been minimized.

Contributor

JeffLIrion commented Nov 16, 2018

Is it ready to be merged once firetv 1.0.7 is released?

@JeffLIrion

This comment has been minimized.

Contributor

JeffLIrion commented Nov 18, 2018

@MartinHjelmare is this ready to be merged once firetv 1.0.7 gets released on pypi?

@MartinHjelmare

This comment has been minimized.

Member

MartinHjelmare commented Nov 18, 2018

I think so. 👍

@JeffLIrion

This comment has been minimized.

Contributor

JeffLIrion commented Nov 18, 2018

firetv 1.0.7 is released on pypi! How can I re-retrigger a build?

@MartinHjelmare

This comment has been minimized.

Member

MartinHjelmare commented Nov 18, 2018

I've restarted the failed job.

@JeffLIrion

This comment has been minimized.

Contributor

JeffLIrion commented Nov 19, 2018

The build passed! Can we merge this PR?

@MartinHjelmare MartinHjelmare merged commit ab8c127 into home-assistant:dev Nov 19, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.04%) to 93.053%
Details

@wafflebot wafflebot bot removed the in progress label Nov 19, 2018

@JeffLIrion

This comment has been minimized.

Contributor

JeffLIrion commented Nov 19, 2018

Thanks @MartinHjelmare! And thanks to @rytilahti for the reviews and @happyleavesaoc for approving my PRs for firetv and releasing new versions on pypi!

mxworm added a commit to mxworm/home-assistant that referenced this pull request Nov 19, 2018

Merge branch 'dev' of https://github.com/home-assistant/home-assistant
…into dev

* 'dev' of https://github.com/home-assistant/home-assistant:
  Bumped ghlocalapi to 0.1.0 (home-assistant#18584)
  Prefix all xiaomi_aqara events (home-assistant#17354)
  Fix MQTT async_added_to_hass (home-assistant#18575)
  Add mikrotik SSL support (home-assistant#17898)
  Reconfigure MQTT binary_sensor component if discovery info is changed (home-assistant#18169)
  Darksky: Expose missing conditions for day 0 forecast (home-assistant#18312)
  Update pyhomematic to 0.1.52 and add features for lights (home-assistant#18499)
  Fix for epson state not updating (home-assistant#18357)
  Support for Point component (home-assistant#17466)
  Template binary sensor to not track all state changes (home-assistant#18573)
  Correct cached stale device tracker handling (home-assistant#18572)
  Add support for sessions (home-assistant#18518)
  Remove turn_on and turn_off feature for clients (home-assistant#18234)
  Log delay and wait_template steps in scripts (home-assistant#18448)
  Logbook speedup (home-assistant#18376)
  Avoid race in entity_platform.async_add_entities() (home-assistant#18445)
  Fix small issue related to topic prefix (home-assistant#18512)
  Enable native support + ADB authentication for Fire TV (home-assistant#17767)
  Re-adding the season attribute (home-assistant#18523)

@balloob balloob referenced this pull request Nov 29, 2018

Merged

0.83 #18776

@KarimGeiger

This comment has been minimized.

Contributor

KarimGeiger commented Dec 4, 2018

Hi - sorry if this is the wrong place to put this, but I can't quite get this component to work with my setup. I've installed adb by running apt-get install android-tools-adb on my HASS server. Then, I connected using adb connect [ip] to the FireTV, which worked flawlessly. Then, after configuring the component (and setting the adbkey path), I get the following errors:

16:27 components/media_player/firetv.py (ERROR)

Couldn't connect to host: [ip]:5555, error: Connection reset by peer
16:27 components/media_player/firetv.py (WARNING)

I'm running version 0.83.3. I've already checked the permissions on the key file and tried starting/stopping the adb server manually by running adb kill-server or adb connecting again. Interestingly, if I'm not mistaken, it sometimes, for a brief moment, seems to work:
screenshot 2018-12-04 at 16 29 10

Thanks for your help, and again, sorry if this isn't the right place.

@MartinHjelmare

This comment has been minimized.

Member

MartinHjelmare commented Dec 4, 2018

Please open an issue if you suspect a bug. If you need help please use our help channels:
https://home-assistant.io/help/#communication-channels

Merged PRs should not be used for support or bug reports. Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Dec 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.