Skip to content
Merged
Changes from 1 commit
Commits
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
113 changes: 110 additions & 3 deletions denonavr/denonavr.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
# pylint: disable=too-many-lines
# pylint: disable=no-else-return

from collections import namedtuple
from collections import (namedtuple, OrderedDict)
from io import BytesIO
import logging
import time
Expand Down Expand Up @@ -37,6 +37,15 @@
"Media Server": "SERVER", "Spotify": "SPOTIFY",
"Flickr": "FLICKR", "Favorites": "FAVORITES"}

SOUND_MODE_MAPPING = OrderedDict([('MUSIC', ['PLII MUSIC']),
('MOVIE', ['PLII MOVIE']),
('GAME', ['PLII GAME']),
('PURE DIRECT', ['DIRECT']),
('AUTO', ['None']),
('DOLBY DIGITAL', ['DOLBY DIGITAL']),
('MCH STEREO', ['MULTI CH STEREO']),
('STEREO', ['STEREO'])])

PLAYING_SOURCES = ("Online Music", "Media Server", "iPod/USB", "Bluetooth",
"Internet Radio", "Favorites", "SpotifyConnect", "Flickr",
"TUNER", "NET/USB", "HDRADIO", "Music Server", "NETWORK",
Expand Down Expand Up @@ -70,6 +79,7 @@
COMMAND_SET_VOLUME_URL = "/goform/formiPhoneAppVolume.xml?1+%.1f"
COMMAND_MUTE_ON_URL = "/goform/formiPhoneAppMute.xml?1+MuteOn"
COMMAND_MUTE_OFF_URL = "/goform/formiPhoneAppMute.xml?1+MuteOff"
COMMAND_SEL_SM_URL = "/goform/formiPhoneAppDirect.xml?MS"

# Zone 2 URLs
STATUS_Z2_URL = "/goform/formZone2_Zone2XmlStatus.xml"
Expand Down Expand Up @@ -105,7 +115,7 @@
"command_power_standby", "command_volume_up",
"command_volume_down", "command_set_volume",
"command_mute_on", "command_mute_off",
"command_netaudio_post"])
"command_sel_sound_mode", "command_netaudio_post"])

DENONAVR_URLS = ReceiverURLs(appcommand=APPCOMMAND_URL,
status=STATUS_URL,
Expand All @@ -123,6 +133,7 @@
command_set_volume=COMMAND_SET_VOLUME_URL,
command_mute_on=COMMAND_MUTE_ON_URL,
command_mute_off=COMMAND_MUTE_OFF_URL,
command_sel_sound_mode=COMMAND_SEL_SM_URL,
command_netaudio_post=COMMAND_NETAUDIO_POST_URL)

ZONE2_URLS = ReceiverURLs(appcommand=APPCOMMAND_URL,
Expand All @@ -141,6 +152,7 @@
command_set_volume=COMMAND_SET_VOLUME_Z2_URL,
command_mute_on=COMMAND_MUTE_ON_Z2_URL,
command_mute_off=COMMAND_MUTE_OFF_Z2_URL,
command_sel_sound_mode=COMMAND_SEL_SM_URL,
command_netaudio_post=COMMAND_NETAUDIO_POST_URL)

ZONE3_URLS = ReceiverURLs(appcommand=APPCOMMAND_URL,
Expand All @@ -159,6 +171,7 @@
command_set_volume=COMMAND_SET_VOLUME_Z3_URL,
command_mute_on=COMMAND_MUTE_ON_Z3_URL,
command_mute_off=COMMAND_MUTE_OFF_Z3_URL,
command_sel_sound_mode=COMMAND_SEL_SM_URL,
command_netaudio_post=COMMAND_NETAUDIO_POST_URL)

POWER_ON = "ON"
Expand Down Expand Up @@ -217,6 +230,9 @@ def __init__(self, host, name=None, show_all_inputs=False, timeout=2.0,
self._input_func = None
self._input_func_list = {}
self._input_func_list_rev = {}
self._sound_mode = None
self._sound_mode_raw = None
self._sound_mode_dict = SOUND_MODE_MAPPING
self._netaudio_func_list = []
self._playing_func_list = []
self._favorite_func_list = []
Expand Down Expand Up @@ -370,7 +386,7 @@ def _update_avr(self):
# pylint: disable=too-many-branches,too-many-statements
# Set all tags to be evaluated
relevant_tags = {"Power": None, "InputFuncSelect": None, "Mute": None,
"MasterVolume": None}
"MasterVolume": None, "selectSurround": None, "SurrMode": None}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (88 > 79 characters)


# Get status XML from Denon receiver via HTTP
try:
Expand Down Expand Up @@ -1032,6 +1048,14 @@ def _get_status_from_xml_tags(self, root, relevant_tags):
elif child.tag == "FriendlyName" and self._name is None:
self._name = child[0].text
relevant_tags.pop(child.tag, None)
elif (child.tag == "selectSurround" or child.tag == "SurrMode"):
sound_mode_raw = child[0].text.rstrip()
self._sound_mode = self.match_sound_mode(sound_mode_raw)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Could you move the match_sound_mode to the sound_mode property?
You won't need the self. _sound_mode_match_warning attribute then.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

just did it.

self._sound_mode_raw = sound_mode_raw
if "selectSurround" in relevant_tags.keys():
relevant_tags.pop("selectSurround", None)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Could you pop both keys from the list, when the first one was found?
They seem to cover the same information. At the moment there might be a second HTTP request, which is not needed, because the STATUS_URL is called first and there is a second call to MAINZONE_URL if some tags are not found yet.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am poping both keys when the first one is found.
when one of the keys is found elif (child.tag == "selectSurround" or child.tag == "SurrMode"): , both keys are poped using relevant_tags.pop("selectSurround", None) and relevant_tags.pop("SurrMode", None).
The if "selectSurround" in relevant_tags.keys(): and if "SurrMode" in relevant_tags.keys(): will in principle always return true, however I put those in to prevent the occasion that later when the code is changed there is no problem with trying to remove a key that does not exist. In the case that if "selectSurround" in relevant_tags.keys(): or if "SurrMode" in relevant_tags.keys(): returns false, the key is not in the list anyway so their is no need to pop it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Therefore I think this part of the code is correct, and a second HTTP request is not possible unless none of the two keys ("selectSurround" and "SurrMode") are found.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ah ok, I was confused by the if statement.
Btw. you don't need the if statements here. If you pop with an default value, there won't be an exception even if the key is not in the list

if "SurrMode" in relevant_tags.keys():
relevant_tags.pop("SurrMode", None)

return relevant_tags

Expand Down Expand Up @@ -1107,6 +1131,26 @@ def input_func_list(self):
"""Return a list of available input sources as string."""
return sorted(self._input_func_list.keys())

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

@property
def sound_mode_list(self):
"""Return a list of available sound modes as string."""
return self._sound_mode_dict.keys()

@property
def sound_mode_dict(self):
"""Return a list of available sound modes as string."""
return self._sound_mode_dict

@property
def sound_mode_raw(self):
"""Return the current sound mode as string as received from the AVR."""
return self._sound_mode_raw

@property
def image_url(self):
"""Return image URL of current playing media when powered on."""
Expand Down Expand Up @@ -1212,6 +1256,69 @@ def set_input_func(self, input_func):
input_func)
return False

@sound_mode.setter
def sound_mode(self, sound_mode):
"""Setter function for sound_mode to switch sound_mode of device."""
self.set_sound_mode(sound_mode)

def set_sound_mode(self, sound_mode):
"""
Set sound_mode of device.

Valid values depend on the device and should be taken from
"sound_mode_list".
Return "True" on success and "False" on fail.
"""
# For selection of sound mode other names then at receiving sound modes
# have to be used
# Therefore source mapping is needed to get sound_mode
# Create command URL and send command via HTTP GET
command_url = self._urls.command_sel_sound_mode + sound_mode
# sent command
try:
if self.send_get_command(command_url):
self._sound_mode = sound_mode
return True
else:
return False
except requests.exceptions.RequestException:
_LOGGER.error("Connection error: sound mode function %s not set.",
sound_mode)
return False

def set_sound_mode_dict(self, sound_mode_dict):
Error_msg = "Syntax of sound mode dictionary not valid, use: OrderedDict([('COMMAND', ['VALUE1','VALUE2'])])"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (117 > 79 characters)

if type(sound_mode_dict) == OrderedDict:
mode_list = list(sound_mode_dict.values())
for sublist in mode_list:
if type(sublist) == list:
for element in sublist:
if type(element) != str:
_LOGGER.error(Error_msg)
return False
else:
_LOGGER.error(Error_msg)
return False
else:
_LOGGER.error(Error_msg)
return False

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

blank line contains whitespace

self._sound_mode_dict = sound_mode_dict
return True

def match_sound_mode(self, sound_mode_raw):
try:
mode_list = list(self._sound_mode_dict.values())

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

When I look at these lines, I get the impression you created the SOUND_MODE_MAPPING and self._sound_mode_dict the wrong way and mixed key and values.
You are using it to get the sound modes from the raw values. For me this is a normal dictionary with each sound_mode_raw as key and its sound_mode as value. That would simplify this part here a lot.
sound_mode_list would be a set set(self._sound_mode_dict.values()) then.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In principle you would be right, and it would be nicer to do it that way if each sound_mode_raw would correspond to exactly 1 matched sound mode.
Unfortunately this is not the case, and multiple sound_mode_raw's can be matched to the same matched sound_mode.

I did think about this before I set it up, and in my opinion it is more elegant to do it like I did it because now I have a single matched sound_mode as key, and then all corresponding raw_sound_modes as list in the value corresponding to that key. In this way I do not need to repeat the matched sound_mode as the value for several raw_sound_mode keys (if you would do it the other way around).

Therefore my method seemed more compact and in my opinion more elegant for this situation.

@starkillerOG starkillerOG Apr 2, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In principle I chose for keeping the self._sound_mode_dict as compact and most eligant as I could make it, and having a bit more complicated code in the match_sound_mode function.

I did this because users of this library and Home Assistant will probably want to tweak the self._sound_mode_dict to have the exact mapping values for their model of receiver, if it is not yet incorperated in the default mapping dict.
However those users will in principle never have to deal with the code of the match_sound_mode function, so it is no problem if that gets a bit more complicated.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It does not look too efficient to replace a dictionary key access by a nested loop across all values of a dictionary during run time.

Maintaining own mapping rules is indeed easier using your way of modelling.

Thus, I would use a dictionary with key: sound_mode_raw and value: sound_mode during run time as I suggested.
This dictionary could be created by the set_sound_mode_dict method you already created from a dictionary with the structure you suggested (key: sound_mode, value: list of sound_mode_raw)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@scarface-4711 smart, it is indead more efficient to use a simple dict key acces during runtime, I have implemented this change in the code.

for sublist in mode_list:
if sound_mode_raw.upper() in sublist:
mode_index = mode_list.index(sublist)
sound_mode = list(self._sound_mode_dict.keys())[mode_index]
return sound_mode
except ValueError:
pass
_LOGGER.warning("Not able to match sound mode, returning raw sound mode.")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (82 > 79 characters)

return sound_mode_raw

def toggle_play_pause(self):
"""Toggle play pause media player."""
# Use Play/Pause button only for sources which support NETAUDIO
Expand Down