Skip to content

Update the Russound RNET component to use enhanced Russound.py #9739

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

Merged
emlove merged 5 commits into from
Oct 21, 2017
Merged

Update the Russound RNET component to use enhanced Russound.py #9739

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1588d61
Updated RussoundRNETDevice.update() to call and enhanced function tha…
altersis Oct 7, 2017
6f55614
Updated RussoundRNETDevice.update() to invoke an enhanced function to…
altersis Oct 7, 2017
83b1bc3
Updated REQUIREMENTS to use version 0.1.9 of the Russound component
altersis Oct 7, 2017
9e4b003
Corrected some minor style details to satisfy Houndbot
altersis Oct 7, 2017
64519f7
Update requirements_all.txt
altersis Oct 19, 2017
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
48 changes: 30 additions & 18 deletions homeassistant/components/media_player/russound_rnet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
CONF_HOST, CONF_PORT, STATE_OFF, STATE_ON, CONF_NAME)
import homeassistant.helpers.config_validation as cv

REQUIREMENTS = ['russound==0.1.7']
REQUIREMENTS = ['russound==0.1.9']

_LOGGER = logging.getLogger(__name__)

Expand Down Expand Up @@ -85,24 +85,36 @@ def __init__(self, hass, russ, sources, zone_id, extra):

def update(self):
"""Retrieve latest state."""
if self._russ.get_power('1', self._zone_id) == 0:
self._state = STATE_OFF
# Updated this function to make a single call to get_zone_info, so that
# with a single call we can get On/Off, Volume and Source, reducing the
# amount of traffic and speeding up the update process.
ret = self._russ.get_zone_info('1', self._zone_id, 4)
_LOGGER.debug("ret= %s", ret)
if ret is not None:

@emlove emlove Oct 19, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Preference:
Our preferred style is the pattern shown below. This saves us from having to indent the rest of the function. This will also line up the indentation of the comments below with the code.

if ret is None:
    _LOGGER.error("Could not update status for zone %s", self._zone_id)
    return

_LOGGER.debug("Updating status for zone %s", self._zone_id)
…

@altersis altersis Oct 20, 2017

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.

Hi @armills , thank you for your feedback. Could it be possible to do the merge like this just for this moment, so that I can concentrate on additional enhancements to the Russound RNET component? Right now this component cannot reconnect if there is a communications error (forcing you to a full Hass restart), and would like to make fixing this my first priority. I will have the update ready in a week from now (including this style change). Do you agree?

@emlove emlove Oct 21, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yep, that's no problem. Not a blocker to merge. Thanks for the contribution! 🎉

_LOGGER.debug("Updating status for zone %s", self._zone_id)
#if self._russ.get_power('1', self._zone_id) == 0:

@houndci-bot houndci-bot Oct 7, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

block comment should start with '# '

if ret[0] == 0:
self._state = STATE_OFF
else:
self._state = STATE_ON

self._volume = ret[2] * 2 / 100.0
#self._volume = self._russ.get_volume('1', self._zone_id) / 100.0

@houndci-bot houndci-bot Oct 7, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

block comment should start with '# '


# Returns 0 based index for source.
#index = self._russ.get_source('1', self._zone_id)

@houndci-bot houndci-bot Oct 7, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

block comment should start with '# '

index = ret[1]
# Possibility exists that user has defined list of all sources.
# If a source is set externally that is beyond the defined list then

@houndci-bot houndci-bot Oct 7, 2017

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 (80 > 79 characters)

# an exception will be thrown.
# In this case return and unknown source (None)
try:
self._source = self._sources[index]
except IndexError:
self._source = None
else:
self._state = STATE_ON

self._volume = self._russ.get_volume('1', self._zone_id) / 100.0

# Returns 0 based index for source.
index = self._russ.get_source('1', self._zone_id)
# Possibility exists that user has defined list of all sources.
# If a source is set externally that is beyond the defined list then
# an exception will be thrown.
# In this case return and unknown source (None)
try:
self._source = self._sources[index]
except IndexError:
self._source = None

_LOGGER.error("Could not update status for zone %s", self._zone_id)

@houndci-bot houndci-bot Oct 7, 2017

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

@property
def name(self):
"""Return the name of the zone."""
Expand Down