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 OpenExchangeRates sensor #2356

Merged
merged 11 commits into from
Jun 25, 2016
1 change: 1 addition & 0 deletions .coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ omit =
homeassistant/components/sensor/nzbget.py
homeassistant/components/sensor/onewire.py
homeassistant/components/sensor/openweathermap.py
homeassistant/components/sensor/openexchangerates.py
homeassistant/components/sensor/plex.py
homeassistant/components/sensor/rest.py
homeassistant/components/sensor/sabnzbd.py
Expand Down
95 changes: 95 additions & 0 deletions homeassistant/components/sensor/openexchangerates.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
"""Support for openexchangerates.org exchange rates service."""
from datetime import timedelta
import logging
import requests
from homeassistant.helpers.entity import Entity
from homeassistant.util import Throttle
from homeassistant.const import CONF_API_KEY

_RESOURCE = 'https://openexchangerates.org/api/latest.json'
_LOGGER = logging.getLogger(__name__)
# Return cached results if last scan was less then this time ago.
MIN_TIME_BETWEEN_UPDATES = timedelta(minutes=100)
CONF_BASE = 'base'
CONF_QUOTE = 'quote'
CONF_NAME = 'name'
DEFAULT_NAME = 'Exchange Rate Sensor'


def setup_platform(hass, config, add_devices, discovery_info=None):
"""Setup the Openexchangerates sensor."""
payload = config.get('payload', None)
rest = OpenexchangeratesData(
_RESOURCE,
config.get(CONF_API_KEY),
config.get(CONF_BASE, 'USD'),
config.get(CONF_QUOTE),
payload
)
rest.update()
add_devices([OpenexchangeratesSensor(rest,
Copy link
Member

Choose a reason for hiding this comment

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

Like I wrote before, please verify that the API key is working before adding it to the system.

Copy link
Contributor Author

@arsaboo arsaboo Jun 23, 2016

Choose a reason for hiding this comment

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

I added the status code check on lines 88 through 90 and the sensor is not added when the API key is not valid, but it does not exit gracefully.

16-06-22 20:46:34 custom_components.sensor.openexchangerates: Check OpenExchangeRates API
16-06-22 20:46:34 homeassistant.components.sensor: Error while setting up platform openexchangerates
Traceback (most recent call last):
  File "/usr/local/lib/python3.4/dist-packages/homeassistant/helpers/entity_component.py", line 98, in _setup_platform
    discovery_info)
  File "/home/pi/.homeassistant/custom_components/sensor/openexchangerates.py", line 32, in setup_platform
    config.get(CONF_QUOTE))])
  File "/home/pi/.homeassistant/custom_components/sensor/openexchangerates.py", line 43, in __init__
    self.update()
  File "/home/pi/.homeassistant/custom_components/sensor/openexchangerates.py", line 64, in update
    self._state = round(value[str(self._quote)], 4)
TypeError: 'NoneType' object is not subscriptable

I should probably do some check around 29, but the try and except did not work. I added the following before rest.update() on line 29, but that did not help.

    try:
        response = requests.get(_RESOURCE, params={'base': config.get(CONF_BASE,
                                                                      'USD'),
                                                   'app_id':
                                                   config.get(CONF_API_KEY)},
                                timeout=10)
    except response.exceptions.HTTPError:
        _LOGGER.error("Check OpenExchangeRates API1")
        return False

Any suggestions? I could not find other sensors that use rest.update() for ideas.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, check the return value at line 29, if false, log error and return. As the http error could happen anytime, you also need to check if value is None, then return, at line 64 before doing anything else with value.

Copy link
Contributor Author

@arsaboo arsaboo Jun 23, 2016

Choose a reason for hiding this comment

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

With an in correct API, it should have triggered the response.exceptions.HTTPError:, but it did not.

If I put a condition not None on 64, other lines give an error as self is None. Is there a way to check if the rest.update() on line 29 or self.update() on line 43 return valid values?

I tried if rest is not None: but that does not work :(

config.get(CONF_NAME, DEFAULT_NAME),
config.get(CONF_QUOTE))])


class OpenexchangeratesSensor(Entity):
"""Implementing the Openexchangerates sensor."""

def __init__(self, rest, name, quote):
"""Initialize the sensor."""
self.rest = rest
self._name = name
self._quote = quote
self.update()

@property
def name(self):
"""Return the name of the sensor."""
return self._name

@property
def state(self):
"""Return the state of the sensor."""
return self._state

@property
def device_state_attributes(self):
"""Return other attributes of the sensor."""
return self.rest.data

def update(self):
"""Update current conditions."""
self.rest.update()
value = self.rest.data
self._state = round(value[str(self._quote)], 4)
Copy link
Member

Choose a reason for hiding this comment

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

This will still error after an HTTP exception below. You can do:

round(value[str(self._quote)], 4) if value is not None else None



# pylint: disable=too-few-public-methods
class OpenexchangeratesData(object):
"""Get data from Openexchangerates.org."""

# pylint: disable=too-many-arguments
def __init__(self, resource, api_key, base, quote, data):
"""Initialize the data object."""
self._resource = resource
self._api_key = api_key
self._base = base
self._quote = quote
self.data = None

@Throttle(MIN_TIME_BETWEEN_UPDATES)
def update(self):
"""Get the latest data from openexchangerates."""
try:
result = requests.get(self._resource, params={'base': self._base,
'app_id':
self._api_key},
timeout=10)
if result.status_code == 200:
self.data = result.json()['rates']
else:
result.raise_for_status()
Copy link
Member

Choose a reason for hiding this comment

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

The idea of raise for status is that you do not have to compare to the status code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quite honestly, we don't need the whole block from line 88 to 91. The requests.exceptions.HTTPError catches 4xx errors. What I am not able to resolve is how to return after rest.update() on line 29 returns nothing so that we don't add_devices?

except requests.exceptions.HTTPError:
_LOGGER.error("Check OpenExchangeRates API")
self.data = None
return False