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

Update vlc.py #5334

Merged
merged 4 commits into from
Jan 15, 2017
Merged

Update vlc.py #5334

merged 4 commits into from
Jan 15, 2017

Conversation

MrMep
Copy link
Contributor

@MrMep MrMep commented Jan 14, 2017

Description:

Added support for additional optional configuration

arguments:

to send to vlc.
It is useful for special configurations of VLC.
For example, I have two sound cards on my server, so I defined two vlc media players:

media_player:
- platform: vlc
  name: speaker_1
  arguments: '--alsa-audio-device=hw:1,0'
- platform: vlc
  name: speaker_2
  arguments: '--alsa-audio-device=hw:0,0'

This way, by specifying the corresponding entity_id, I can send the output to the desired speaker.
It is also useful for TTS.

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

Example entry for configuration.yaml (if applicable):

media_player:
- platform: vlc
  name: speaker_1
  arguments: '--alsa-audio-device=hw:1,0'

Checklist:

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

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

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New 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:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

Added support for additional optional configuration

arguments:

to send to vlc.
It is useful for special configurations of VLC.
For example, I have two sound cards on my server, so I defined two vlc media players:

media_player:
- platform: vlc
  name: speaker_1
  arguments: '--alsa-audio-device=hw:1,0'
- platform: vlc
  name: speaker_2
  arguments: '--alsa-audio-device=hw:0,0'

This way, by specifying the corresponding entity_id, I can send the output to the desired speaker.
It is also useful for TTS.
@mention-bot
Copy link

@MrMep, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Danielhiversen and @armills to be potential reviewers.

})


# pylint: disable=unused-argument
def setup_platform(hass, config, add_devices, discovery_info=None):
"""Setup the vlc platform."""
add_devices([VlcDevice(config.get(CONF_NAME))])
add_devices([VlcDevice(config.get(CONF_NAME), config.get(ADDITIONAL_ARGS))])

Choose a reason for hiding this comment

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

line too long (80 > 79 characters)

@Danielhiversen
Copy link
Member

Great, thanks
Could you update the documentation?

@@ -20,28 +20,31 @@

_LOGGER = logging.getLogger(__name__)

ADDITIONAL_ARGS = 'arguments'
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be renamed to CONF_ARGUMENTS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, done!

@MrMep
Copy link
Contributor Author

MrMep commented Jan 14, 2017

Documentation updated, I hope I got it right!

Added default value for "arguments"
@Danielhiversen Danielhiversen merged commit 9fff634 into home-assistant:dev Jan 15, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Apr 30, 2017
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.

6 participants