Skip to content

Fire events on homekit TV remote key press#29588

Merged
bdraco merged 8 commits intohome-assistant:devfrom
nickw444:nwhyte/homekit-tv-media-keys-events
May 19, 2020
Merged

Fire events on homekit TV remote key press#29588
bdraco merged 8 commits intohome-assistant:devfrom
nickw444:nwhyte/homekit-tv-media-keys-events

Conversation

@nickw444
Copy link
Copy Markdown
Contributor

@nickw444 nickw444 commented Dec 7, 2019

Description:

Follow up to #26671 with a different approach (event based) based on feedback provided.

This PR adds support for TV's (with device_class: tv) to have events emitted when a key is pressed within the apple remote widget. These events can be connected in automations to control the TV in whatever way is appropriate for the user's use-case.

Since there is no existing service on media_player for generic button navigation/presses (and I think that is a whole can of worms), this PR adds a way for users to control their media devices via the iOS remote widget.

Without this PR, the remote currently only supports play, pause, volume up and volume down. This PR does not change this, but does fire events for these buttons too.

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

Example entry for configuration.yaml (if applicable):

homeassistant:
  customize:
    media_player.my_smart_tv:
      device_class: tv

homekit:

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.
  • I have followed the development checklist

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

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

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

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

@nickw444
Copy link
Copy Markdown
Contributor Author

Hey @MartinHjelmare I noticed you have recently reviewed some homekit PRs, do you think you'd be able to take a look at this PR?

Comment thread homeassistant/components/homekit/type_media_players.py Outdated
Comment thread homeassistant/components/homekit/type_media_players.py
Comment thread tests/components/homekit/test_type_media_players.py Outdated
@stale
Copy link
Copy Markdown

stale Bot commented Mar 26, 2020

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale Bot added the stale label Mar 26, 2020
@nickw444
Copy link
Copy Markdown
Contributor Author

This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.

Nooo stale bot, I promise I am coming back to this! 😱 Hopefully this weekend 🤞

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Apr 10, 2020

@nickw444. I’m going to be working in this area soon. Mostly a heads up in case you want to get this PR in before and avoid having to work out a conflict

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Apr 13, 2020

#30479

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Apr 19, 2020

@nickw444 I'm done the changes I mentioned so there shouldn't be any new conflicts. I fixed the conflict that did pop up (it will need an isort/black run)

@MartinHjelmare MartinHjelmare self-assigned this Apr 19, 2020
@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 9, 2020

@MartinHjelmare Noticed you self assigned this. I'm thinking about a different approach that avoids the custom events in #35421

@MartinHjelmare
Copy link
Copy Markdown
Member

I've just assigned myself since I've done a review.

@bdraco bdraco mentioned this pull request May 9, 2020
20 tasks
Comment thread homeassistant/components/homekit/type_media_players.py Outdated
@nickw444
Copy link
Copy Markdown
Contributor Author

@MartinHjelmare Noticed you self assigned this. I'm thinking about a different approach that avoids the custom events in #35421

Hey @bdraco it's not immediately clear to me whether that PR makes this one obsolete (as it's closed). Should I continue to push forward with this? I've locked in some time this weekend to look into this if so 😄

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 14, 2020

@MartinHjelmare Noticed you self assigned this. I'm thinking about a different approach that avoids the custom events in #35421

Hey @bdraco it's not immediately clear to me whether that PR makes this one obsolete (as it's closed). Should I continue to push forward with this? I've locked in some time this weekend to look into this if so 😄

Please continue with this PR. Thank you!

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 16, 2020

@nickw444 You can find the lines with missing test coverage with pytest --cov=homeassistant/components/homekit/ --cov-report term-missing -- tests/components/homekit/test_*.py

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be merged when build passes.

Comment thread tests/components/homekit/test_type_media_players.py
Copy link
Copy Markdown
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggested change inline, and the CI should pass.

Co-authored-by: J. Nick Koston <nick@koston.org>
@nickw444 nickw444 requested a review from bdraco May 19, 2020 07:10
@bdraco bdraco merged commit a51372f into home-assistant:dev May 19, 2020
@nickw444 nickw444 deleted the nwhyte/homekit-tv-media-keys-events branch May 20, 2020 01:19
@lock lock Bot locked and limited conversation to collaborators May 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

missing volume control for homekit media player

4 participants