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

Remove km from visibility, add visibility_distance #8454

Merged
merged 5 commits into from
Jul 14, 2017
Merged

Remove km from visibility, add visibility_distance #8454

merged 5 commits into from
Jul 14, 2017

Conversation

gollo
Copy link
Contributor

@gollo gollo commented Jul 12, 2017

Description:

The values for visibility provided by the Met Office are not in km but are codes (VG=Very Good, etc). These then map to estimated distance ranges of visibility.

  • Remove the "km" unit from visibility as it makes no sense.
  • Add new sensor value visibility_distance which is a mapping of the 2 letter code to a range in km (as linked above). This is a new optional configuration value.

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

Example entry for configuration.yaml (if applicable):

  - platform: metoffice
    api_key: "APIKEY"
    monitored_conditions:
      ...
      - visibility_distance
      ....

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

@homeassistant
Copy link
Contributor

Hi @gollo,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

Special case for added visibility_distance which is just a lookup on visibility code,
not retrieved from the dp call directly.
"""
if self._condition == "visibility_distance" and 'visibility' in self.data.data.__dict__.keys():

Choose a reason for hiding this comment

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

line too long (103 > 79 characters)

@@ -119,6 +129,12 @@ def name(self):
@property
def state(self):
"""Return the state of the sensor."""
"""
Special case for added visibility_distance which is just a lookup on visibility code,

Choose a reason for hiding this comment

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

line too long (96 > 79 characters)

not retrieved from the dp call directly.
"""
if self._condition == "visibility_distance" \
and 'visibility' in self.data.data.__dict__.keys():

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

@@ -119,6 +129,14 @@ def name(self):
@property
def state(self):
"""Return the state of the sensor."""
"""
Special case for added visibility_distance

Choose a reason for hiding this comment

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

trailing whitespace

not retrieved from the dp call directly.
"""
if (self._condition == 'visibility_distance' and
'visibility' in self.data.data.__dict__.keys()):

Choose a reason for hiding this comment

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

visually indented line with same indent as next logical line

which is just a lookup on visibility code,
not retrieved from the dp call directly.
"""
if (self._condition == 'visibility_distance' and

Choose a reason for hiding this comment

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

trailing whitespace

Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

Thanks 🐬

@balloob balloob merged commit d473f34 into home-assistant:dev Jul 14, 2017
@balloob balloob added this to the 0.49 milestone Jul 14, 2017
balloob pushed a commit that referenced this pull request Jul 16, 2017
* Remove km from visibility, add visibility_distance

* Fix line length

* Fix trailing space and line break indentation

* Indentation

* More whitespace
dethpickle pushed a commit to dethpickle/home-assistant that referenced this pull request Aug 18, 2017
* Remove km from visibility, add visibility_distance

* Fix line length

* Fix trailing space and line break indentation

* Indentation

* More whitespace
@home-assistant home-assistant locked and limited conversation to collaborators Oct 20, 2017
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