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

opensky sensor #7061

Merged
merged 3 commits into from
Apr 20, 2017
Merged

opensky sensor #7061

merged 3 commits into from
Apr 20, 2017

Conversation

happyleavesaoc
Copy link
Contributor

@happyleavesaoc happyleavesaoc commented Apr 11, 2017

Description:

Report the number of flights in a given area via the OpenSky Network, and raise events for flights crossing the area boundary.

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

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: opensky
    radius: 10   # kilometers
    name: my area  # optional
    latitude: <latitude>  # optional, defaults to home zone latitude
    longitude: <longitude>  # optional, defaults to home zone longitude

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 files were added to .coveragerc.

@mention-bot
Copy link

@happyleavesaoc, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @robbiet480 to be potential reviewers.

# pylint: disable=unused-argument
def setup_platform(hass, config, add_devices, discovery_info=None):
"""Setup the Open Sky platform."""
session = requests.Session()
Copy link
Member

Choose a reason for hiding this comment

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

Move that into OpenSkySensor.__init__

@property
def unit_of_measurement(self):
"""Unit of measurement."""
return UNIT_OF_MEASUREMENT
Copy link
Member

Choose a reason for hiding this comment

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

Don't use a const if you need it only on one place.

@property
def icon(self):
"""Icon."""
return ICON
Copy link
Member

Choose a reason for hiding this comment

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

Don't use a const if you need it only on one place.

@happyleavesaoc
Copy link
Contributor Author

Addressed comments.

@Br3nda
Copy link

Br3nda commented Apr 15, 2017

i gave this a test, with this config

sensor:
  - platform: opensky
    radius: 10   # kilometers

and got this error

Traceback (most recent call last):
  File "/usr/lib/python3.5/asyncio/tasks.py", line 241, in _step
    result = coro.throw(exc)
  File "/usr/local/lib/python3.5/dist-packages/homeassistant/helpers/entity_component.py", line 359, in async_process_entity
    new_entity, self, update_before_add=update_before_add
  File "/usr/local/lib/python3.5/dist-packages/homeassistant/helpers/entity_component.py", line 189, in async_add_entity
    yield from self.hass.loop.run_in_executor(None, entity.update)
  File "/usr/lib/python3.5/asyncio/futures.py", line 361, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/lib/python3.5/asyncio/tasks.py", line 296, in _wakeup
    future.result()
  File "/usr/lib/python3.5/asyncio/futures.py", line 274, in result
    raise self._exception
  File "/usr/lib/python3.5/concurrent/futures/thread.py", line 55, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/home/brenda/.homeassistant/custom_components/sensor/opensky.py", line 109, in update
    if distance > self._radius:
TypeError: unorderable types: NoneType() > float()

@happyleavesaoc
Copy link
Contributor Author

@Br3nda do you have a lat/long set in your homeassistant config?

@Br3nda
Copy link

Br3nda commented Apr 16, 2017

@happyleavesaoc yes - it is set.
It did this error first time -- and again an hour later.
Maybe you can log what the api returned when it throws an exception like this?

@happyleavesaoc
Copy link
Contributor Author

@Br3nda Ah ok, so it was an intermittent error? There's probably occasionally a flight with bad lat/long data. I added an extra check to prevent the error on the latest commit. Thanks for the feedback!


_LOGGER = logging.getLogger(__name__)

SCAN_INTERVAL = timedelta(seconds=12) # opensky public limit is 10 seconds
Copy link
Member

Choose a reason for hiding this comment

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

I think that we should be friendly and not being too close to the limit of the API. Isn't it enough to get the data every 30 or so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, if you want to know when a flight enters your region, it'll be gone already in 30 seconds by the time the sensor updates.

But it is a SCAN_INTERVAL, so people with that use case could always set it lower if we default to 30.

@balloob balloob dismissed pvizeli’s stale review April 20, 2017 05:56

Comments addressed

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.

Nice work 🐬

@balloob balloob merged commit 1860b6c into home-assistant:dev Apr 20, 2017
@balloob balloob mentioned this pull request Apr 21, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Aug 12, 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.

8 participants