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

Sound mode support media_player general and denonavr #13706

Closed
wants to merge 9 commits into from
Closed

Sound mode support media_player general and denonavr #13706

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
062db8a
Add sound mode support
starkillerOG Apr 5, 2018
69440e2
Denonavr to v.0.7.0
starkillerOG Apr 5, 2018
8dab888
Add sound mode support
starkillerOG Apr 5, 2018
142a71a
Add sound mode support
starkillerOG Apr 5, 2018
c7df1bb
Add sound mode support
starkillerOG Apr 5, 2018
a5ae281
Added sound_mode_raw as a state attribute
starkillerOG May 3, 2018
90924ac
update sound mode raw property
starkillerOG May 3, 2018
e4da0d4
spacing
starkillerOG May 3, 2018
29659cb
spacing
starkillerOG May 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions homeassistant/components/media_player/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@

SERVICE_PLAY_MEDIA = 'play_media'
SERVICE_SELECT_SOURCE = 'select_source'
SERVICE_SELECT_SOUND_MODE = 'select_sound_mode'
SERVICE_CLEAR_PLAYLIST = 'clear_playlist'

ATTR_MEDIA_VOLUME_LEVEL = 'volume_level'
Expand All @@ -78,6 +79,8 @@
ATTR_APP_NAME = 'app_name'
ATTR_INPUT_SOURCE = 'source'
ATTR_INPUT_SOURCE_LIST = 'source_list'
ATTR_SOUND_MODE = 'sound_mode'
ATTR_SOUND_MODE_LIST = 'sound_mode_list'
ATTR_MEDIA_ENQUEUE = 'enqueue'
ATTR_MEDIA_SHUFFLE = 'shuffle'

Expand Down Expand Up @@ -106,6 +109,7 @@
SUPPORT_CLEAR_PLAYLIST = 8192
SUPPORT_PLAY = 16384
SUPPORT_SHUFFLE_SET = 32768
SUPPORT_SELECT_SOUND_MODE = 65536
Copy link
Member

Choose a reason for hiding this comment

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

We should not add this as a new supported feature for media players. We should only add things that exist among many media players.

Revert all changes to this file and add a new service to the denonavr platform called media_player.denonavr_set_sound_mode to allow changing thet mode.

Copy link
Member

Choose a reason for hiding this comment

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

I was planning to add support for sound modes in a form or another to songpal, so that'd be two platforms for using such a feature. I haven't checked out yet how to do that, but will try to find some time during the upcoming weekend for it, if that's fine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@balloob I asked about this on the form when I first started with this code, and at that time there was also someone who was intrested in implementing sound mode support for the sonos platform based on my code (but he wanted to wait until I finished and it has been quite a long time).

But that would already make 3 platforms that would uses this.
Besides almost all sound systems have some kind of sound mode feature, therefore it could be implemented in most of the platforms if someone is willing to make the code (and has that device).

Therfore I made this a general support feature in the media_player/init.py file, and I think this is a good thing. Do you agree @balloob?

Copy link
Member

Choose a reason for hiding this comment

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

See https://developers.home-assistant.io/docs/en/entity_index.html#changing-the-entity-model

Copy link
Member

Choose a reason for hiding this comment

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

So let me close this PR and the frontend PR, and you can open an issue on the architecture repo to propose the new feature.


# Service call validation schemas
MEDIA_PLAYER_SCHEMA = vol.Schema({
Expand All @@ -129,6 +133,10 @@
vol.Required(ATTR_INPUT_SOURCE): cv.string,
})

MEDIA_PLAYER_SELECT_SOUND_MODE_SCHEMA = MEDIA_PLAYER_SCHEMA.extend({
vol.Required(ATTR_SOUND_MODE): cv.string,
})

MEDIA_PLAYER_PLAY_MEDIA_SCHEMA = MEDIA_PLAYER_SCHEMA.extend({
vol.Required(ATTR_MEDIA_CONTENT_TYPE): cv.string,
vol.Required(ATTR_MEDIA_CONTENT_ID): cv.string,
Expand Down Expand Up @@ -164,6 +172,9 @@
SERVICE_SELECT_SOURCE: {
'method': 'async_select_source',
'schema': MEDIA_PLAYER_SELECT_SOURCE_SCHEMA},
SERVICE_SELECT_SOUND_MODE: {
'method': 'async_select_sound_mode',
'schema': MEDIA_PLAYER_SELECT_SOUND_MODE_SCHEMA},
SERVICE_PLAY_MEDIA: {
'method': 'async_play_media',
'schema': MEDIA_PLAYER_PLAY_MEDIA_SCHEMA},
Expand Down Expand Up @@ -194,6 +205,8 @@
ATTR_APP_NAME,
ATTR_INPUT_SOURCE,
ATTR_INPUT_SOURCE_LIST,
ATTR_SOUND_MODE,
ATTR_SOUND_MODE_LIST,
ATTR_MEDIA_SHUFFLE,
]

Expand Down Expand Up @@ -343,6 +356,17 @@ def select_source(hass, source, entity_id=None):
hass.services.call(DOMAIN, SERVICE_SELECT_SOURCE, data)


@bind_hass
def select_sound_mode(hass, sound_mode, entity_id=None):
"""Send the media player the command to select sound mode."""
data = {ATTR_SOUND_MODE: sound_mode}

if entity_id:
data[ATTR_ENTITY_ID] = entity_id

hass.services.call(DOMAIN, SERVICE_SELECT_SOUND_MODE, data)


@bind_hass
def clear_playlist(hass, entity_id=None):
"""Send the media player the command for clear playlist."""
Expand Down Expand Up @@ -387,6 +411,8 @@ def async_service_handler(service):
params['position'] = service.data.get(ATTR_MEDIA_SEEK_POSITION)
elif service.service == SERVICE_SELECT_SOURCE:
params['source'] = service.data.get(ATTR_INPUT_SOURCE)
elif service.service == SERVICE_SELECT_SOUND_MODE:
params['sound_mode'] = service.data.get(ATTR_SOUND_MODE)
elif service.service == SERVICE_PLAY_MEDIA:
params['media_type'] = \
service.data.get(ATTR_MEDIA_CONTENT_TYPE)
Expand Down Expand Up @@ -569,6 +595,16 @@ def source_list(self):
"""List of available input sources."""
return None

@property
def sound_mode(self):
"""Name of the current sound mode."""
return None

@property
def sound_mode_list(self):
"""List of available sound modes."""
return None

@property
def shuffle(self):
"""Boolean if shuffle is enabled."""
Expand Down Expand Up @@ -712,6 +748,17 @@ def async_select_source(self, source):
"""
return self.hass.async_add_job(self.select_source, source)

def select_sound_mode(self, sound_mode):
"""Select sound mode."""
raise NotImplementedError()

def async_select_sound_mode(self, sound_mode):
"""Select sound mode.

This method must be run in the event loop and returns a coroutine.
"""
return self.hass.async_add_job(self.select_sound_mode, sound_mode)

def clear_playlist(self):
"""Clear players playlist."""
raise NotImplementedError()
Expand Down Expand Up @@ -785,6 +832,11 @@ def support_select_source(self):
"""Boolean if select source command supported."""
return bool(self.supported_features & SUPPORT_SELECT_SOURCE)

@property
def support_select_sound_mode(self):
"""Boolean if select sound mode command supported."""
return bool(self.supported_features & SUPPORT_SELECT_SOUND_MODE)

@property
def support_clear_playlist(self):
"""Boolean if clear playlist command supported."""
Expand Down
80 changes: 71 additions & 9 deletions homeassistant/components/media_player/denonavr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,32 @@
from homeassistant.components.media_player import (
SUPPORT_PAUSE, SUPPORT_NEXT_TRACK, SUPPORT_PREVIOUS_TRACK,
SUPPORT_TURN_OFF, SUPPORT_VOLUME_MUTE, SUPPORT_VOLUME_STEP,
SUPPORT_SELECT_SOURCE, SUPPORT_PLAY_MEDIA, MEDIA_TYPE_CHANNEL,
MediaPlayerDevice, PLATFORM_SCHEMA, SUPPORT_TURN_ON,
MEDIA_TYPE_MUSIC, SUPPORT_VOLUME_SET, SUPPORT_PLAY)
SUPPORT_SELECT_SOURCE, SUPPORT_SELECT_SOUND_MODE,
SUPPORT_PLAY_MEDIA, MEDIA_TYPE_CHANNEL, MediaPlayerDevice,
PLATFORM_SCHEMA, SUPPORT_TURN_ON, MEDIA_TYPE_MUSIC,
SUPPORT_VOLUME_SET, SUPPORT_PLAY)
from homeassistant.const import (
CONF_HOST, STATE_OFF, STATE_PLAYING, STATE_PAUSED,
CONF_NAME, STATE_ON, CONF_ZONE, CONF_TIMEOUT)
import homeassistant.helpers.config_validation as cv

REQUIREMENTS = ['denonavr==0.6.1']
REQUIREMENTS = ['denonavr==0.7.0']

_LOGGER = logging.getLogger(__name__)

DEFAULT_NAME = None
DEFAULT_SHOW_SOURCES = False
DEFAULT_TIMEOUT = 2
DEFAULT_SOUND_MODE = True
CONF_SHOW_ALL_SOURCES = 'show_all_sources'
CONF_ZONES = 'zones'
CONF_SOUND_MODE = 'sound_mode'
CONF_SOUND_MODE_DICT = 'sound_mode_dict'
CONF_VALID_ZONES = ['Zone2', 'Zone3']
CONF_INVALID_ZONES_ERR = 'Invalid Zone (expected Zone2 or Zone3)'
KEY_DENON_CACHE = 'denonavr_hosts'

ATTR_SOUND_MODE_RAW = 'sound_mode_raw'

SUPPORT_DENON = SUPPORT_VOLUME_STEP | SUPPORT_VOLUME_MUTE | \
SUPPORT_TURN_ON | SUPPORT_TURN_OFF | \
SUPPORT_SELECT_SOURCE | SUPPORT_VOLUME_SET
Expand All @@ -49,6 +54,8 @@
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
vol.Optional(CONF_HOST): cv.string,
vol.Optional(CONF_NAME): cv.string,
vol.Optional(CONF_SOUND_MODE, default=DEFAULT_SOUND_MODE): cv.boolean,
vol.Optional(CONF_SOUND_MODE_DICT): vol.Schema({str: list}),
vol.Optional(CONF_SHOW_ALL_SOURCES, default=DEFAULT_SHOW_SOURCES):
cv.boolean,
vol.Optional(CONF_ZONES):
Expand Down Expand Up @@ -84,6 +91,10 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
else:
add_zones = None

# Get config option for sound mode
sound_mode_support = config.get(CONF_SOUND_MODE)
sound_mode_dict = config.get(CONF_SOUND_MODE_DICT)

# Start assignment of host and name
new_hosts = []
# 1. option: manual setting
Expand Down Expand Up @@ -117,7 +128,9 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
show_all_inputs=show_all_sources, timeout=timeout,
add_zones=add_zones)
for new_zone in new_device.zones.values():
receivers.append(DenonDevice(new_zone))
receivers.append(DenonDevice(new_zone,
sound_mode_support,
sound_mode_dict))
cache.add(host)
_LOGGER.info("Denon receiver at host %s initialized", host)

Expand All @@ -129,7 +142,7 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
class DenonDevice(MediaPlayerDevice):
"""Representation of a Denon Media Player Device."""

def __init__(self, receiver):
def __init__(self, receiver, sound_mode_support, sound_mode_dict):
"""Initialize the device."""
self._receiver = receiver
self._name = self._receiver.name
Expand All @@ -147,6 +160,24 @@ def __init__(self, receiver):
self._frequency = self._receiver.frequency
self._station = self._receiver.station

self._sound_mode_support = sound_mode_support
if sound_mode_support:
self._sound_mode = self._receiver.sound_mode
self._sound_mode_raw = self._receiver.sound_mode_raw
if sound_mode_dict is None:
self._sound_mode_list = self._receiver.sound_mode_list
else:
self._receiver.set_sound_mode_dict(sound_mode_dict)
self._sound_mode_list = list(sound_mode_dict)
else:
self._sound_mode = None
self._sound_mode_raw = None
Copy link
Contributor

Choose a reason for hiding this comment

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

self._sound_mode_raw attribute does not seem to be used at all and could be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just made a commit where I implemented it as a state property.
Thanks for the feedback, I ment to implement this but forgot about it.
It is usefull to have this in the state because the raw sound mode provides more detailed information. (can be used in automations or to troubleshoot if the raw sound mode is not in the mapping in the denonavr library).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did not test this change yet, will test it tonight.

self._sound_mode_list = None

self._supported_features_base = SUPPORT_DENON
self._supported_features_base |= (sound_mode_support and
SUPPORT_SELECT_SOUND_MODE)

def update(self):
"""Get the latest status information from device."""
self._receiver.update()
Expand All @@ -164,6 +195,9 @@ def update(self):
self._band = self._receiver.band
self._frequency = self._receiver.frequency
self._station = self._receiver.station
if self._sound_mode_support:
self._sound_mode = self._receiver.sound_mode
self._sound_mode_raw = self._receiver.sound_mode_raw

@property
def name(self):
Expand Down Expand Up @@ -197,12 +231,27 @@ def source_list(self):
"""Return a list of available input sources."""
return self._source_list

@property
def sound_mode(self):
"""Return the current matched sound mode."""
return self._sound_mode

@property
def sound_mode_raw(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as for self._sound_mode_raw. There is no equivalent property in constructor of parent media_player class, thus I assume it is not used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as above

"""Return the current raw sound mode."""
return self._sound_mode_raw

@property
def sound_mode_list(self):
"""Return a list of available sound modes."""
return self._sound_mode_list

@property
def supported_features(self):
"""Flag media player features that are supported."""
if self._current_source in self._receiver.netaudio_func_list:
return SUPPORT_DENON | SUPPORT_MEDIA_MODES
return SUPPORT_DENON
return self._supported_features_base | SUPPORT_MEDIA_MODES
return self._supported_features_base

@property
def media_content_id(self):
Expand Down Expand Up @@ -276,6 +325,15 @@ def media_episode(self):
"""Episode of current playing media, TV show only."""
return None

@property
def device_state_attributes(self):
"""Return device specific state attributes."""
attributes = {}
if self._sound_mode_raw is not None and self._sound_mode_support\
and self._power == 'ON':
Copy link

Choose a reason for hiding this comment

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

continuation line missing indentation or outdented

attributes[ATTR_SOUND_MODE_RAW] = self._sound_mode_raw
return attributes

def media_play_pause(self):
"""Simulate play pause media player."""
return self._receiver.toggle_play_pause()
Expand All @@ -292,6 +350,10 @@ def select_source(self, source):
"""Select input source."""
return self._receiver.set_input_func(source)

def select_sound_mode(self, sound_mode):
"""Select sound mode."""
return self._receiver.set_sound_mode(sound_mode)

def turn_on(self):
"""Turn on media player."""
if self._receiver.power_on():
Expand Down
10 changes: 10 additions & 0 deletions homeassistant/components/media_player/services.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,16 @@ select_source:
description: Name of the source to switch to. Platform dependent.
example: 'video1'

select_sound_mode:
description: Send the media player the command to change sound mode.
fields:
entity_id:
description: Name(s) of entities to change sound mode on.
example: 'media_player.marantz'
sound_mode:
description: Name of the sound mode to switch to.
example: 'Music'

clear_playlist:
description: Send the media player the command to clear players playlist.
fields:
Expand Down
2 changes: 1 addition & 1 deletion requirements_all.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ defusedxml==0.5.0
deluge-client==1.0.5

# homeassistant.components.media_player.denonavr
denonavr==0.6.1
denonavr==0.7.0

# homeassistant.components.media_player.directv
directpy==0.2
Expand Down