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
Merged

Add OpenExchangeRates sensor #2356

merged 11 commits into from
Jun 25, 2016

Conversation

arsaboo
Copy link
Contributor

@arsaboo arsaboo commented Jun 23, 2016

Description: Initial version to obtain current exchange rate between any two currency pair from OpenExchangeRates.

Obtain API key here. Free accounts allow 1000 requests per month and is limited to 'USD' base currency.

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

Example entry for configuration.yaml (if applicable):

sensor:
  platform: openexchangerates
  api_key: your_api_key
  base: USD #optional
  quote: EUR
  name: USDEUR #optional

Checklist:

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

If code communicates with devices:

  • 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.

Get the current exchange rate between any two supported currency pair.

_LOGGER.error("Check OpenExchangeRates API")
self.data = None
return False

Choose a reason for hiding this comment

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

farcy v1.1

  • 1: W391 blank line at end of file

@arsaboo arsaboo mentioned this pull request Jun 23, 2016
8 tasks
def update(self):
"""Get the latest data from openexchangerates."""
try:
result = requests.get(self._resource + '?base=' + self._base +
Copy link
Contributor

Choose a reason for hiding this comment

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

You can pass a params dict instead of encoding the query string yourself: http://docs.python-requests.org/en/master/user/quickstart/#passing-parameters-in-urls

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks....done :)

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 :(

@arsaboo
Copy link
Contributor Author

arsaboo commented Jun 24, 2016

I was able to resolve the issue :). Now, the sensor checks the API key for any errors and adds the sensor if there are no errors.

I have updated the documentation as well, but just wanted to inform you that I am still not able to get the tox thing working on my end. If someone can help me with that (which I prefer so that I can do that myself in the future) or can carry out the tests for me, I will really appreciate that.

@balloob
Copy link
Member

balloob commented Jun 25, 2016

Looks good! 🐬

@balloob balloob merged commit 2ac752d into home-assistant:dev Jun 25, 2016
"""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

@arsaboo
Copy link
Contributor Author

arsaboo commented Jun 25, 2016

Super....thanks all!!

@arsaboo arsaboo deleted the patch-2 branch June 25, 2016 15:52
@home-assistant home-assistant locked and limited conversation to collaborators Mar 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants