Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add device condition support for media_player #28161

Merged
merged 2 commits into from Oct 25, 2019
Merged

Add device condition support for media_player #28161

merged 2 commits into from Oct 25, 2019

Conversation

shmooey
Copy link
Contributor

@shmooey shmooey commented Oct 24, 2019

Description:

Adds basic device condition support for media_player integration - on, off, idle, paused, playing.

Related issue (if applicable): fixes #27004

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 does not interact with devices:

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

@project-bot project-bot bot added this to Needs review in Dev Oct 24, 2019
@shmooey
Copy link
Contributor Author

shmooey commented Oct 24, 2019

Is there anything I need to do in home-assistant-polymer? When I went to see what it looked like in the UI, the condition dropdown items were all blank for me.

image

Also, what kind of documentation should I add, if any?

@balloob
Copy link
Member

balloob commented Oct 24, 2019

Great first contribution 👍 Run script/translations_develop to get UI strings to work.

balloob
balloob approved these changes Oct 24, 2019
View reviewed changes
Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

This looks great ! 🎉

Welcome to the Home Assistant community 🐬

Dev automation moved this from Needs review to Reviewer approved Oct 24, 2019
@balloob
Copy link
Member

balloob commented Oct 24, 2019

This can be merged when you format your code with Black so the tests pass 👍

You can run black path/to/file.py

@shmooey
Copy link
Contributor Author

shmooey commented Oct 25, 2019

The formatting error was caused by #28171, I've rebased this branch so the Black issue is fixed now 😊

@balloob
Copy link
Member

balloob commented Oct 25, 2019

Oh snap, didn't realize. Wish Black would give some more output so we know where to look.

Great work ! 🐬

@balloob balloob merged commit 2c914e0 into home-assistant:dev Oct 25, 2019
Dev automation moved this from Reviewer approved to Done Oct 25, 2019
@lock lock bot locked and limited conversation to collaborators Oct 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@balloob balloob balloob approved these changes

Assignees
No one assigned
Labels
cla-signed core Hacktoberfest has-tests integration: media_player
Projects
Dev
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add device condition support to the media_player integration
3 participants
@shmooey @balloob @homeassistant