Skip to content

Add missing configuration variables#11390

Merged
fabaff merged 3 commits into
devfrom
metoffice
Jan 6, 2018
Merged

Add missing configuration variables#11390
fabaff merged 3 commits into
devfrom
metoffice

Conversation

@fabaff
Copy link
Copy Markdown
Member

@fabaff fabaff commented Dec 30, 2017

Description:

Add missing configuration variables.

Related issue (if applicable): fixes 37359

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

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: metoffice
    api_key: !secret met_office
    name: Weather
    monitored_conditions:
      - weather
      - temperature
      - feels_like_temperature
      - wind_speed

Checklist:

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

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

"""Initialize the data object."""
self._hass = hass
self._datapoint = datapoint
self._hass = hass
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hass isn't used.

data = MetOfficeCurrentData(hass, datapoint, site)
try:
data.update()
except (ValueError, dp.exceptions.APIException) as err:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we should reraise the error in data.update. It's already logged there and raising it again will add an ugly stack trace to the log of it happens after setup_platform is finished.

Instead we can check if data.data is None, which it shouldn't be if everything is successful.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We can do that.

def device_state_attributes(self):
"""Return the state attributes of the device."""
attr = {}
attr = dict()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why change this? I think the former is cleaner.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's faster too, btw.

'visibility' in self.data.data.__dict__.keys()):
return VISIBILTY_CLASSES.get(self.data.data.visibility.value)
if self._condition in self.data.data.__dict__.keys():
if self._condition in self.data.data.__dict__:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can use:

if hasattr(self.data.data, self._condition):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hasattr is faster as __dict__ 👍

def state(self):
"""Return the state of the sensor."""
if (self._condition == 'visibility_distance' and
'visibility' in self.data.data.__dict__.keys()):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use:

hasattr(self.data.data, 'visibility'):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hasattr is faster as __dict__ 👍

@fabaff fabaff merged commit c613585 into dev Jan 6, 2018
@fabaff fabaff deleted the metoffice branch January 6, 2018 20:53
@balloob balloob mentioned this pull request Jan 11, 2018
@home-assistant home-assistant locked and limited conversation to collaborators May 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants