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

Enable _uasyncio module #6041

Merged
merged 10 commits into from
Feb 25, 2022
Merged

Enable _uasyncio module #6041

merged 10 commits into from
Feb 25, 2022

Conversation

t-ikegami
Copy link

This PR fixes ticks in moduasyncio.c to work with adafruit version of asyncio, and enables _uasyncio module. With this PR, C-version of Task and TaskQueue are used in the asyncio.

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

The _uasyncio implementation of Task does not provide__await___(). Our internal implementation of async/await (done by @WarriorOfWire) requires that. Otherwise code is going to fail. I had to add __await__() in the Python version in a couple of places:
https://github.com/adafruit/Adafruit_CircuitPython_asyncio/blob/28ef812f14ef1fde1036eb83a4878bd14aa9a67d/asyncio/task.py#L152
https://github.com/adafruit/Adafruit_CircuitPython_asyncio/blob/28ef812f14ef1fde1036eb83a4878bd14aa9a67d/asyncio/core.py#L52-L53

I started to add __await__(), but because __iter__() is provided in a special way, I had not yet figured out how to make it work, and so did not use _uasyncio initially.

In addition, I would like to rename _uasyncio to _asyncio, but that is minor.

@t-ikegami
Copy link
Author

Thank you for the comment. Now I understand why _uasyncio is missing in the modules. I'll close this PR.

@t-ikegami t-ikegami closed this Feb 16, 2022
@dhalbert
Copy link
Collaborator

If you can see how to add __await()__, that would be great. but it's rather deep in method resolution. I haven't had time to look further.

@dhalbert
Copy link
Collaborator

I appreciate your work, and we can use these commits in the future, in any case.

@dhalbert dhalbert mentioned this pull request Feb 16, 2022
@t-ikegami t-ikegami reopened this Feb 22, 2022
@t-ikegami
Copy link
Author

t-ikegami commented Feb 22, 2022

The __await__ method is added to Task class, which is just a wrapper of getiter. The name of the module is not modified for now, because the renaming should be in sync with adafruit/asyncio/core.py. With this PR, Task object becomes awaitable, and the following code works.

import asyncio

async def coro(s) :
    print("coro start", s)
    await asyncio.sleep(s)
    print("coro done", s)
    return f"coro result({s})"

async def main() :
    print("main start")
    res = await asyncio.gather(coro(1), coro(2), coro(3))
    print(res)
    print("main end")

asyncio.run(main())

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Thank you for doing this, and testing it! I have a few changes, but I'm glad you were able to figure out an easy way to do this.

extmod/moduasyncio.c Outdated Show resolved Hide resolved
py/objmodule.c Outdated Show resolved Hide resolved
Register _asyncio module in CP manner.
@t-ikegami
Copy link
Author

t-ikegami commented Feb 23, 2022

The module is renamed from _uasyncio to _asyncio.

The task_getiter and task_iternext are moved back to the original position. A prototype definition of task_getiter is added.

I couldn't figure out MICROPY_PORT_BUILTIN_MODULES stuff well, but the module is now registered not in objmodule.c but by MP_REGISTER_MODULE. (I referred modure.c and modujson.c.)

@t-ikegami t-ikegami requested a review from dhalbert February 24, 2022 14:31
Copy link
Author

@t-ikegami t-ikegami left a comment

Choose a reason for hiding this comment

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

I appreciate the clarification.

@dhalbert
Copy link
Collaborator

I appreciate the clarification.

You were right to use MP_REGISTER_MODULE. The comments re MICROPY_PORT_BUILTIN_MODULES were obsolete. I am having trouble getting it to pass one test. Not quite there yet.

@t-ikegami
Copy link
Author

I am having trouble getting it to pass one test. Not quite there yet.

I suspect that tests/unix/extra_coverage.py.exp should be updated (_asyncio module may appear in repl auto-completion), though I failed to build unix/micropython-coverage so far.

@dhalbert
Copy link
Collaborator

dhalbert commented Feb 25, 2022

I suspect that tests/unix/extra_coverage.py.exp should be updated (_asyncio module may appear in repl auto-completion), though I failed to build unix/micropython-coverage so far.

Yes, that was it. I had to sudo apt install pkgconf libffi-dev to get micropython-coverage to build:
make VARIANT=coverage -j12 # or whatever -j is appropriate for you

Copy link
Author

@t-ikegami t-ikegami left a comment

Choose a reason for hiding this comment

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

Thank you for the update.

dhalbert
dhalbert previously approved these changes Feb 25, 2022
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

After my last set of tweaks, I tested this with a two-task blinking LEDs program on a Metro M4, and it's working. Thanks for this, @t-ikegami !

@dhalbert
Copy link
Collaborator

I have to shrink the matrixportal build slightly, or perhaps turn off uasyncio in that build.

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

Successfully merging this pull request may close these issues.

2 participants