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

Async support #64

Closed
cdce8p opened this issue Mar 22, 2018 · 22 comments
Closed

Async support #64

cdce8p opened this issue Mar 22, 2018 · 22 comments

Comments

@cdce8p
Copy link
Contributor

cdce8p commented Mar 22, 2018

Currently HAP-python uses threads which, at least on a larger scale, does impact performance quite a bit. Are there plans to change to an async framework, like asyncio in the future?

Unfortunately this would also require a more recent min python version, to take full advantage of the new async syntax introduced with py3.5.

@ikalchev
Copy link
Owner

This is actually a very good idea, which I think we might pursue. I was recently thinking on the threads starting to be an issue and it looks like asyncio can fit the bill nicely.

Dropping 3.4 will not be a huge issue I believe. I had similar requests before, but no good reasons to do it.

I am currently busier at work than what is usual, but I expect things to be back to normal from this weekend onwards. I will definitely spend some time on this soon and try to make some first steps. Of course, you are welcome to do further suggestions and PRs if you want.

On a side note, @jslay88 started a chat for the project on discord I think recently which I hadn't had the time to "take over" yet. I will try to kick this off as well.

Best,
Ivan

@jslay88
Copy link
Contributor

jslay88 commented Mar 22, 2018

https://discord.gg/BhxZ4Dy

So lonely :)

@cdce8p
Copy link
Contributor Author

cdce8p commented Mar 22, 2018

Good to here that. I'm happy to help, although I just started watching talks and tutorials about this two weeks ago 😅 I didn't wanted to start before knowing if this is something you want. It's quite some work and will require major changes to the code base.

Maybe we can coordinate efforts on the discord channel than

@ikalchev
Copy link
Owner

FYI started transforming the Accessory/Bridge run methods to use asyncio (seemed easiest starting point)

@cdce8p
Copy link
Contributor Author

cdce8p commented Mar 27, 2018

👍 Could you open a [WIP] PR for it?

@ikalchev
Copy link
Owner

Yes, good point, I am doing this now since I moved to dev and master branches. I hadn't had anything significant to push recently due to lack of time.

@ikalchev
Copy link
Owner

@cdce8p have you used aiohttp https://aiohttp.readthedocs.io/en/stable/

I am considering it as the next step (after the Accessories run method).

@cdce8p
Copy link
Contributor Author

cdce8p commented Mar 31, 2018

@ikalchev Not yet, since I didn't have/had any experience with networking prior to working with HAP-python. aiohttp seems to be the right choice though.

Did you open a WIP PR? That would really help. Especially to know what will be changed and to give suggestions.

@ikalchev
Copy link
Owner

I will, I just need to try something before the PR, I am learning asyncio as well

@ikalchev
Copy link
Owner

By the way what is WIP?

@cdce8p
Copy link
Contributor Author

cdce8p commented Mar 31, 2018

Work in progress

@cdce8p
Copy link
Contributor Author

cdce8p commented Apr 2, 2018

@ikalchev @thomaspurchas What do you guys think about creating a separate branch, maybe dev-async to support the async conversion. For now all async related PR's would be based against this branch, new features from dev merged via PR to dev-async and when we are ready we merge it into dev (later master) with the complete async support.

That way we don't need to guaranty that every version of the conversion works. Ultimately this would allow us to work more freely than if we were bound to just dev.

@thomaspurchas I don't think to merge #76 into a feature branch (ultimately a PR) is a wise idea here.

@ikalchev If you want, I could support this from an administrative standpoint as well.

@thomaspurchas
Copy link

thomaspurchas commented Apr 2, 2018 via email

@ikalchev
Copy link
Owner

ikalchev commented Apr 4, 2018

Honestly, I would like to shortcut this and do it in dev. I think that the change is not that big code-wise, but could hide bugs for ages. Keeping it in an async-dev branch will not help to uncover these bugs, because there won't be much traffic there.

I think with more unit tests things can go in fairly stable in dev. (e.g. I got a few when I wrote units for the driver in my latest commit) What do you guys think?

@cdce8p
Copy link
Contributor Author

cdce8p commented Apr 4, 2018

I would be ok with that if you thinks so. Before you merge any async PR into dev, could you do a release to 1.1.9 after #73?

@ikalchev
Copy link
Owner

ikalchev commented Apr 4, 2018

Sure, we can wait for 73, push it to master, "release" and then finish with the asyncio.

@rcarmo
Copy link

rcarmo commented Aug 20, 2018

Hi there. I'm unsure as to the state of this, since I see references to asyncio already throughout the code.

I'm looking for a simple example of how to run the AccessoryDriver inside an event loop so that I can have concurrent handling of other network traffic (I'm trying to turn an SSDP/UPNP service into an on/off switch, essentially).

Is there a simple asyncio example already?

@ikalchev
Copy link
Owner

I think @cdce8p can give a more in-depth example from the Home-Assistant community, but here is a quick one (just added the async keyword in the run method, otherwise its just the sensor from the accessories folder):

import random

from pyhap.accessory import Accessory
from pyhap.const import CATEGORY_SENSOR


class TemperatureSensor(Accessory):

    category = CATEGORY_SENSOR

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)

        serv_temp = self.add_preload_service('TemperatureSensor')
        self.char_temp = serv_temp.configure_char('CurrentTemperature')

    @Accessory.run_at_interval(3)
    async def run(self):
        '''This decorator will basically await asyncio.sleep(3) before re-running this code'''
        self.char_temp.set_value(random.randint(18, 26))

Basically, if the run method of the Accessory is marked async, it will not run in a separate thread, but in the event loop. I.e. you can use asyncio in the run as it is ran in the event loop.

You can queue "jobs" in the event loop by calling self.driver.async_add_job within the Accessory (the method is in accessory_driver.py, go ahead an have a look).

Let me know if you have need anything else.

@ikalchev
Copy link
Owner

By the way, closing this as the majority of the work in the core is done (Thanks for the fine work @cdce8p)

@rcarmo
Copy link

rcarmo commented Aug 21, 2018

Thanks, in the meantime I figured most of that out for non-async (i.e., threaded) by reading through the source code. I realize that you actually test for the target being a coroutine in add_job, etc. (which feels too magic and un-pythonic, but works), and got a similar example to work:

   @Accessory.run_at_interval(15)
    def run(self):
        self.driver.async_add_job(chromecast_on, CHROMECAST_NAME, self)

# in another module:

def chromecast_on(name, accessory):
    log.info("starting discovery")
    devices = discover(DISCOVERY_URN, timeout=1, retries=2)
    for d in devices:
        caps = get_capabilities(d.location)
        if caps.get('device', None):

# still in another module:

def discover(service, timeout=5, retries=1, mx=3):
    group = ("239.255.255.250", 1900)
    message = "\r\n".join([
        'M-SEARCH * HTTP/1.1',
        'HOST:{0}:{1}',
        'MAN:"ssdp:discover"',
        'ST:{st}','MX:{mx}','',''])
    socket.setdefaulttimeout(timeout)
    responses = {}
    for _ in range(retries):
        sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, socket.IPPROTO_UDP)

But what I do not understand just yet is where I can change the default event loop, or how to initialize the driver correctly for using asyncio together with uvloop (since I also need to use another set of routines that are fully async).

Is there a full example with a driver being initialized with set_event_loop_policy?

@cdce8p
Copy link
Contributor Author

cdce8p commented Aug 25, 2018

@ikalchev Still a few things to do, but I haven't had any time to do it since. I'm ok with closing this though.

@rcarmo I haven't used uvloop, but you should be able to pass your loop to the AccessoryDriver during initialization with loop=loop. That should work.

@ikalchev
Copy link
Owner

BTW I plan to include uvloop as an extra so you can do pip install HAP-python[uvloop].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants