Add Freebox device tracker

[stilllman]

Description:

This pull request adds a device tracker platform for the Freebox, the server/router of the french FAI Free, along the lines of PR #10702. It is based on freepybox, but it necessitates some changes that have not been released yet.

The platform can be set up automatically through discovery (once this netdisco commit will be released), or configured as usual.

There is some documentation in the header of the file, I can make that a full documentation page in home-assistant.github.io if need be.

Example entry for configuration.yaml (if applicable):

device_tracker:
  - platform: freebox
    host: foobar.fbox.fr
    port: 1234

Checklist:

• The code change is tested and works locally.

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

• Documentation added/updated in home-assistant.github.io

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.

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.

[houndci-bot]

'homeassistant.components.device_tracker.DOMAIN' imported but unused

[stilllman]

I've run tox in my local repo, but I have no output so I'm not sure it's actually testing anything...

I have some questions regarding this PR:

First of all, is it ok to make this pull request to initiate discussions even if it depends on a third-party library version that is not released yet, or should I have waited?

Secondly, the Freebox requires a manual intervention from the user to authorize applications, pretty much like the Hue bridge does: the first time an application requests access to the server, the user must authorize it directly on the device. What would be the correct way to handle this? Do I need to make a custom component so that the user can configure the access from the home page, as is done for Hue for instance, or is it acceptable to simply give instructions in the documentation (for starters)?

About the automatic setup through discovery, I was wondering if it is something desirable for device trackers? Device tracking can be seen as a bit intrusive, is it a good idea to start doing it without user intervention? If not, what would be the correct approach to notify the user that we discovered a Freebox device, along with the host and port to use in her configuration file if she wishes to set it up?

[MartinHjelmare]

WIP PRs are OK.

You can use the configurator component to ask the user to perform actions to facilitate login or setup. Look at eg the August component.

Another alternative is the new config entry flow. See #12830 for hue. This flow is very new and might be considered experimental. It also requires test coverage if you implement it.

The config flow approach will also handle discovery and let the user dismiss a discovered device if its not wanted to be setup as an integration.

The user can turn off discovery component if it's not wanted. It's also possible to configure it to ignore certain platforms.
https://home-assistant.io/components/discovery/

[ylecuyer]

Is there a way to beta test ?

[stilllman]

@MartinHjelmare The new config flow seems to be exactly what I had in mind, I'll read through the related PRs and see if I can implement it.

@ylecuyer Beta testing would be much welcomed, but for now it requires a bit of manual intervention to install the unreleased-yet dependencies. You will need to install freepybox with the latest changes, either from the upstream repo, or from my fork where the version number has already been bumped to 0.0.3. If you want to test discovery, you will need a version of netdisco with this commit, or you can rollback the netdisco version number to 1.2.4 and use manual configuration. Please let me know if you have any questions or feedback!

[stilllman]

It's been a while since I reached out to the maintainer of Freepybox without response, so I decided to fork his work in an asynchronous library, which will be more suited to integration in Hass. I've released it on PyPI (aiofreepybox) and I've modified the component to make it asynchronous as well.

However, I still have an error I'm not sure how to solve: my log repeatedly indicates that "async_update_info was never awaited". As far as I understand, it should be called and awaited by async_track_time_interval (or another function down the line), right? I also have another error that may or may not be related:

2018-04-28 01:34:16 ERROR (MainThread) [homeassistant.core] Error doing job: Exception in callback _ProactorReadPipeTransport._loop_reading(<_OverlappedF...ed result=b''>)
Traceback (most recent call last):
File "c:\python36\lib\asyncio\events.py", line 145, in _run
self._callback(*self._args)
File "c:\python36\lib\asyncio\proactor_events.py", line 188, in _loop_reading
self._closing)
AssertionError
2018-04-28 01:34:16 INFO (MainThread) [homeassistant.core] Bus:Handling <Event system_log_event[L]: timestamp=1524872056.4453623, level=ERROR, message=Error doing job: Exception in callback _ProactorReadPipeTransport._loop_reading(<_OverlappedF...ed result=b''>), exception=Traceback (most recent call last):
File "c:\python36\lib\asyncio\events.py", line 145, in _run
self._callback(*self._args)
File "c:\python36\lib\asyncio\proactor_events.py", line 188, in _loop_reading
self._closing)
AssertionError
, source=c:\users\luc_t_000\projects\home-assistant\homeassistant\core.py>

I have this error even when I disable the Freebox device tracker, so I'm not sure it comes from my code, but since it seems to relate to the asyncio loop, I'm wondering if it could be related to the other error. Any insight? FYI, I'm currently working on Windows with Python 3.6.4.

[stilllman]

I've tested my code on Ubuntu with Python 3.5.3: I no longer have the error in asyncio, but I still have the "coroutine was never awaited" one, so I guess I'm doing something wrong with async_track_time_interval.

[ylecuyer]

Shouldn't async_update_info be decorated with @asyncio.coroutine ?

If you search for async_track_time_interval they follow this pattern

[MartinHjelmare]

Do we need to create a new instance of Freepybox at every update? Can't we instantiate this in init and save it as an instance attribute?

[stilllman]

Sure, I'll do that. Do you see any problem if I leave the session opening/closing in the update function though? Doing so, we don't have to deal explicitely with lost connection, auth token refresh etc.

[MartinHjelmare]

The future that this returns is never awaited.

[MartinHjelmare]

Since we're not interested in the result of the future, ie the return value of async_see, I suggest you use asyncio.wait instead of asyncio.gather.

[stilllman]

Right, fixed

[MartinHjelmare]

Why do we need the try... finally?

[MartinHjelmare]

If getting the hosts blows up, nothing in the following normal flow will execute, including awaiting the closing task.

[stilllman]

The goal was to "fire and forget" the closing task when an exception occurs (push the async task to the event loop, but without waiting for it to finish before rethrowing the exception), and "fire and await" in the normal flow, but I may have misunderstood how this works (this is my first time developing with asyncio).

Perhaps I should wrap this in an async context manager, I'll need to check how to do that properly.

[houndci-bot]

'asyncio' imported but unused

[MartinHjelmare]

asyncio is part of standard library so should be imported at the top of the module.

[MartinHjelmare]

asyncio is part of standard library so should be imported at the top of the module.

[MartinHjelmare]

Same as above.

[MartinHjelmare]

Move to top of module.

[MartinHjelmare]

If we expect an exception here, maybe we should try to catch it, if we know what it is and if it will happen often?

[MartinHjelmare]

Move the rest of the docstring from this line to the documentation. Replace this line with a link to the documentation. See other platforms for the correct link format.

[stilllman]

@MartinHjelmare Thanks for the reviews, all good points! I will be on holidays for the next week, I'll make the changes when I'll be back.

However, I still don't understand why async_update_info is never awaited, should I decorate it with @asyncio.coroutine instead of using async def, like @ylecuyer proposed? Looking at the code of async_track_time_interval, it seems to me that both versions should be supported, but then again I'm no asyncio expert.

[MartinHjelmare]

No, we should use async def from now on when defining coroutine functions.

Are you still getting that warning after the latest changes?

[stilllman]

@MartinHjelmare After investigating a bit, I think I understand what the problem is. The Throttle decorator wraps the coroutine function in a non-coroutine function (and rightfully so, asyncio.iscoroutinefunction returns False for bound methods decorated with @Throttle), so async_add_job ends up running it in an executor instead of wrapping it in a Task. I'll add a non-decorated method that calls the throttled one to work around this, unless you have a better idea?

[MartinHjelmare]

Since you're scheduling the update from this module, I suggest simply taking the max of configured scan interval and min time between updates, and use that as update interval. And remove the throttle.

[stilllman]

Documentation added in home-assistant/home-assistant.io#5415.

[MartinHjelmare]

Put this and the see call within an else: here. These calls should not be done if the error happens.

[MartinHjelmare]

Add a check if last_results is truthy before doing this.

[syssi]

home-assistant/home-assistant.io#5415

[gsemet]

Hi ! This component works great :) @stilllman have you tried to add internet status support (sensor I guess?), in order to see which state is the connection (1,2 ... to 6), connexion uptime, current speed dl/up,... ?

[stilllman]

Hey @gsemet! I have considered making a full-fledged component instead of just a device tracker, with support for toggling wifi, getting list of missed calls, etc., but I wanted to see if there was a need in the community before doing so :)

I won't start working on this before at least a few weeks though, I just had a happy event in my life that will keep me quite busy! In the meantime, how about creating a feature request with the features you would like to see in the component? You can take a look at the Freebox API to see what would be feasible or not.

[gsemet]

Congrat for this happy event :) mine is expected by the end of this year, so I may find some time to implement the freebox status + some action switchs ;)