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.

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

Add Alexa.ChannelController functions for media players #27671

Merged
merged 18 commits into from
Oct 23, 2019

Conversation

Dilbert66
Copy link
Contributor

@Dilbert66 Dilbert66 commented Oct 15, 2019

Added missing Alexa.ChannelController functions. Specifically Change Channel and SkipChannel commands. These functions will call the play_media function in a media_player app if it has the capability published and pass on the channel# or channel name. The selected media player can then use this to select the channel on the device it is associated to.

Modified the existing Alexa.StepSpeaker Setvolume function to actually do a stepped volume change using the steps sent by Alexa. The Alexa default step of 10 for a simple volume up/down can be changed via an exposed media_player attribute called volume_step_default. The default is set to 1.
Any other value than default will be sent as a looped sequence of volume up/down calls to the media_player.

Modifed the test script test_smart_home.py to add the missing payload attribute volumeStepDefault and test for it. Also added ChannelController tests.

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][dev-checklist]

If the code does not interact with devices:

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

Checks the Channel controller box in #24579

@probot-home-assistant
Copy link

Hey there @home-assistant/cloud, @ochlocracy, mind taking a look at this pull request as its been labeled with a integration (alexa) you are listed as a codeowner for? Thanks!

@Dilbert66
Copy link
Contributor Author

This is a a resubmit of my previous pull request. Due to issues with my dev environment , I felt it best to start fresh. I now have the tox , flake8, pylint programs running correctly.

@MartinHjelmare MartinHjelmare changed the title Added missing Alexa.ChannelController functions for media players. Modifed StepSpeaker to do incremental stepped volume up/down Add missing Alexa.ChannelController functions for media players Oct 15, 2019
@Dilbert66 Dilbert66 force-pushed the smarthome branch 2 times, most recently from f28809c to 6653f57 Compare October 15, 2019 16:58
@Dilbert66 Dilbert66 changed the title Add missing Alexa.ChannelController functions for media players Add Alexa.ChannelController functions for media players Oct 17, 2019
@tulindo
Copy link
Contributor

tulindo commented Oct 17, 2019

Hi @Dilbert66 I've seen your code and looks good...
Just a comment: You implemented the volumestep and channelskip functions using a loop. What about adding a sleep between every volumeup/nexttrack command for safety?

@Dilbert66
Copy link
Contributor Author

A safety delay is a good idea actually. There is processing delay induced with the call but I agree, i will add it.

@Dilbert66
Copy link
Contributor Author

Thank you. I've made the recommended changes , re-tested and commited.

@Dilbert66 Dilbert66 force-pushed the smarthome branch 2 times, most recently from 0ff29d5 to 6302b5d Compare October 20, 2019 16:01
Dev automation moved this from Needs review to Reviewer approved Oct 23, 2019
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.

Excellent! I had just merged another PR for Alexa so there are some merge conflicts.
This PR can be merged when the conflicts have been resolved.

Great first contribution! 🐬 Welcome to the Home Assistant community 🎉

…hannel

and SkipChannel commands. These functions will call the play_media function
 in a media_player app if it has the capability published and pass on the
  channel# or channel name. The selected media player can then use this to
   select the channel on the device it is associated to.
Modified the existing Alexa.StepSpeaker Setvolume function to actually do
a stepped volume change using the steps sent by Alexa. The Alexa default
 step of 10 for a simple volume up/down can be changed via an exposed
 media_player attribute called volume_step_default.
 The default is set to 1. Any other value then default will be sent
 as sequential volume up /down to the media_player.
…d to surround them in quotes for the tests to pass properly.
…ut the behavior in the handler. The test suite does not like multiple await calls in a loop. Will investigate further. The handler code works though.
Added test for callSign payload attribute.
@Dilbert66
Copy link
Contributor Author

Thank you! Conflicts resolved and re-pushed. I see that the Python37 checks are failing but from the looks of the azure logs, this looks to be an issue outside of this pull request.

@balloob balloob merged commit b1a3740 into home-assistant:dev Oct 23, 2019
Dev automation moved this from Reviewer approved to Done Oct 23, 2019
@balloob
Copy link
Member

balloob commented Oct 23, 2019

🎉

@Dilbert66 Dilbert66 deleted the smarthome branch October 23, 2019 15:54
@lock lock bot locked and limited conversation to collaborators Oct 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants