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

[Bug] await wait(0) does not yield (enough) #1429

Closed
laurensvalk opened this issue Feb 8, 2024 · 5 comments · Fixed by pybricks/pybricks-micropython#263
Closed

[Bug] await wait(0) does not yield (enough) #1429

laurensvalk opened this issue Feb 8, 2024 · 5 comments · Fixed by pybricks/pybricks-micropython#263
Labels
bug Something isn't working software: pybricks-blocks Issues with blocks and code generation software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: multitasking Issues releated to parallel threads, tasks, coroutines, etc.

Comments

@laurensvalk
Copy link
Member

Describe the bug
The following program does not run the second stack of program, i.e. the buttons won't operate the gripper.

image

To reproduce

from pybricks.hubs import InventorHub
from pybricks.parameters import Button, Direction, Port, Stop
from pybricks.pupdevices import Motor, Remote
from pybricks.tools import multitask, run_task, wait

# Set up all devices.
inventor_hub = InventorHub()
gripper = Motor(Port.E, Direction.CLOCKWISE)
remote = Remote(timeout=None)

async def main1():
    print('Hello, Pybricks!')
    while True:
        await wait(0) # < ---- automatically inserted, but not enough

async def main2():
    while True:
        await wait(0)
        if Button.LEFT in remote.buttons.pressed():
            await gripper.run_target(500, 80)
        elif Button.RIGHT in remote.buttons.pressed():
            await gripper.run_target(500, 0)
        else:
            await wait(50)


async def main():
    await multitask(main1(), main2())

run_task(main())

Expected behavior
Bad loops are caught by the block language to automatically yield. This is working, but apparently await wait(0) does not currently appear to yield at least once when it should.

The workaround would be to insert wait(1) but it would be better to fix it in the firmware.

@laurensvalk laurensvalk added bug Something isn't working topic: multitasking Issues releated to parallel threads, tasks, coroutines, etc. software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) labels Feb 8, 2024
@laurensvalk
Copy link
Member Author

laurensvalk commented Feb 19, 2024

So this program will not print anything.

from pybricks.tools import multitask, run_task, wait

async def main1():
    while True:
        await wait(0)

async def main2():
    while True:
        await wait(0)
        print('Hello!')
        await wait(500)


async def main():
    await multitask(main1(), main2())

run_task(main())

image

@laurensvalk
Copy link
Member Author

laurensvalk commented Feb 19, 2024

We can simplify this and rule out the multitask function as the culprit because it still fails without it:

from pybricks.tools import run_task, wait

async def main1():
    while True:
        await wait(0) # second taks only proceeds with nonzero delay

async def main2():
    while True:
        await wait(0)
        print('Hello!')
        await wait(500)

async def main():
    
    first = main1()
    second = main2()

    while True:
        print("first")
        next(first)
        print("second") # never gets here, so main1 is blocking
        next(second)

run_task(main())

@laurensvalk
Copy link
Member Author

laurensvalk commented Feb 19, 2024

So await wait(0) just doesn't yield.

So we could either

  1. Make it yield once
  2. Use await wait(1)

I kind of want to avoid 2 because it would mean simple math loops would unnecessarily take 1 ms around the loop.

But when looking at this, the loop time doesn't seem to work as intended which is a separate issue.

EDIT: Assuming this issue is fixed, it's going to be a 10ms roundtrip around the loop anyway, so maybe we should just use the 1 ms here.

@laurensvalk laurensvalk added the software: pybricks-blocks Issues with blocks and code generation label Feb 19, 2024
@dlech
Copy link
Member

dlech commented Feb 19, 2024

My gut reaction here is to make await wait(0) yield once (pretty sure this is what await asyncio.sleep(0) does) and do #1460 (comment).

@laurensvalk
Copy link
Member Author

Fixes suggested in pybricks/pybricks-micropython#262, pybricks/pybricks-micropython#263.

While I was thinking about this, it occurred to me we could extend this by making all awaitables yield at least once. For example await sensor.reflection() now only yields if there is a mode change to be done. By making it always yield once, any loop with at least one await statement would be safe. But since Python doesn't do this, I think it is probably not good to teach this pattern, and so we shouldn't do this. And it would slow down all those statements by one extra iteration. End of mental note.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working software: pybricks-blocks Issues with blocks and code generation software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: multitasking Issues releated to parallel threads, tasks, coroutines, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants