Multitasking #166

laurensvalk · 2023-05-05T19:21:23Z

This is an early exploration to implement pybricks/support#917. See that issue for rationale and motivation. We can use this pull request to dig more into the technical details.

I normally like to review it commit-by-commit, but in this case the commits are more for historical context. The later commits pretty much rewrite the earlier ones based on lessons learned over time.

Work in progress:

This pull request enables running scripts like the following. Scripts without run_task should run without user-facing changes.

from pybricks.hubs import PrimeHub
from pybricks.pupdevices import Motor
from pybricks.parameters import Port, Direction, Stop, Color
from pybricks.robotics import DriveBase
from pybricks.tools import wait, multitask, run_task

# Initialize devices as usual. You can do this on any hub.
hub = PrimeHub()
left_motor = Motor(Port.A, Direction.COUNTERCLOCKWISE)
right_motor = Motor(Port.B)
drive_base = DriveBase(left_motor, right_motor, wheel_diameter=56, axle_track=112)

# Task to drive in a square.
async def square():
    for side in range(4):
        # Drive forward and then turn.
        await drive_base.straight(200)
        await drive_base.turn(90)

# Task to blink.
async def blink():
    for i in range(10):
        hub.light.on(Color.RED)
        await wait(500)
        hub.light.on(Color.GREEN)
        await wait(500)

# Hello world task.
async def hello(name):
    print("Hello!")
    await wait(2000)
    print(name)

# This is the main program
async def main():
    print("Running multiple tasks at once!")
    await multitask(square(), blink(), hello("Pybricks"))
    print("You can also just run one task.")
    await hello("World!")

# Run the main program.
run_task(main())

coveralls · 2023-05-05T19:26:33Z

Coverage: 54.517% (+1.6%) from 52.872% when pulling 4b939b0 on multitasking into 28c95d3 on master.

dlech · 2023-05-05T19:53:10Z

The module must be named tasks or I will be certain to go crazy. 😝

All other pybricks modules are plural like this: tools, hubs, parameters, *devices, etc.

I would also like to have task available for use as a local variable name.

laurensvalk · 2023-05-06T08:33:08Z

Thanks, this made me consider an alternative way to possibly do it. It's added to the PR. This avoids the module naming question altogether, so if we like this variant we can save the module naming philosophy for a call 😉

Since all and race used previously create task objects, another way is to use a Task type instead of a task module.

Task(*args, race=False):

    Creates a new task that runs one or more tasks in parallel.

    By default, this completes when all given tasks have finished.

    Choose race=True to exit once the first task completes and cancel all other tasks.

This allows the exact same use cases but fits in better with the rest of our API. It's also exactly how it is implemented.

Borrowing from the example we'd get:

from pybricks.tools import Task, run_task

# This is the main task
async def main():
    print("Running multiple tasks at once!")
    await Task(square(), blink(), hello("Pybricks"))
    print("You can also just run one task.")
    await hello("World!")

# Run the main task.
run_task(main())

You could even take this a step further by letting run() be a method or class method:

from pybricks.tools import Task     # One less import this way

# If it was a method.
Task(main()).run()

# If it was a class method.
Task.run(main())

But I figured that a separate function called run_task would perhaps be easier to follow for beginners:

Task() creates a task;
run_task runs it.

dlech · 2023-05-06T19:45:39Z

Fewer modules and functions is always a good thing.

Why does the replacement for all/race need to be a class?

Alternate naming ideas:

from pybricks.tools import multitask, run_async

...

async def main():
    await multitask(do_thing(), do_other_thing())

run_async(main())

Implementation detail ideas:

Nested run loops are not allowed. Calling run_async() inside of another run_async() should raise an exception with a helpful error message.
Calling multitask() when no run loop is running should raise an exception with a helpful error message.

laurensvalk · 2023-05-06T20:34:55Z

It doesn’t strictly have to be a class, I’m just thinking it probably does not need to be a module that is imported from another module, as done in earlier drafts. It seems that we mainly did that to nicely group run/all/race with some sort of common namespace.

But with something like race as a kwarg to a Task class or a multitask function as suggested above, we shouldn’t need that. This occurred to me while doing the implementation since all/race use the same code for the most part.

For what it’s worth, it is a class in that it creates a new task that runs a set of given tasks in parallel. It is also implemented as a class. It instantiates an object that you can await.

But we could indeed make a wrapper function that gives you an instance of that class. In the C implementation this could be done by just giving the type a lower case name so we don’t have to add an explicit wrapper function.

I like the lowercase names you suggested though, so I might pull those in. Indeed, helpful errors would make this easier too.

laurensvalk · 2023-05-08T09:03:55Z

On sensors, we were hoping to avoid await on read methods, but maybe most would need it after all.

This is because they do not only have mode switches, but also writes for light controls.

So except for maybe the force sensor, pretty much everything would need awaits.

github-actions · 2023-05-08T10:35:13Z

Download the artifacts for this pull request:

dlech · 2023-05-08T14:05:42Z

This is because they do not only have mode switches, but also writes for light controls.

Isn't that a separate method from reading though?

Also, lights could be reworked to use a background process for I/O similar to how the built-in hub lights with SPI or I2C busses already work.

dlech · 2023-05-08T14:14:41Z

lights could be reworked

Now that we have finalizers enabled, I think it would be fairly easy to drop the "external" light implementation and use the "internal" light implementation with dynamically allocated light structures.

laurensvalk · 2023-05-08T14:26:36Z

Isn't [light] a separate method from reading though?

Reading after setting the lights means switching back to one of the measuring modes.

dlech · 2023-05-08T15:43:31Z

I guess async/blocking get value methods would work better for I2C sensors on EV3/NXT too since we could perform the I2C transaction right then instead of having a background polling process.

laurensvalk · 2023-05-09T07:02:27Z

I guess async/blocking get value methods would work better for I2C sensors on EV3/NXT too since we could perform the I2C transaction right then instead of having a background polling process.

Indeed. I've come to think that it is justifiable to use await for anything that uses communication with a sensor.

I think we may just have to make the documentation very explicit about it for each method that needs it.

Currently working on some ideas to generalize the common awaitable type to implement this relatively cheaply.

laurensvalk · 2023-05-12T07:50:10Z

I've done some additional cleanups and refactoring. This should make it easy to add async support for sensors and pb_task used by Bluetooth functions next week.

dlech

This is looking really good.

I think the most difficult problem remaining is solving cancellation/closing that takes some time. In Python, this is solved using async with, but I don't the we want to have to write that all of the time. In asyncio.streams, there is a wait_closed() method for this sort of thing, but again an extra call that would be nice to avoid.

dlech · 2023-05-18T16:34:12Z

lib/pbio/include/pbio/control.h

+     * Flag indicating controller should run until it stalls. Can be used to
+     * explicitly run until it stalls to find a mechanism endpoint.
+     */
+    PBIO_CONTROL_TYPE_FLAG_OBJECTIVE_IS_STALL = 1 << 3,
 } pbio_control_type_t;


This could be made a bit safer to use by leaving the existing enum as-is and adding instead packing the data using a new:

typedef struct { pbio_control_type_t type : 2; bool stop_on_stall : 1; bool objective_is_stall : 1; } pbio_control_type_and_flags_t

dlech · 2023-05-18T16:43:27Z

pybricks/common/pb_type_control.c

@@ -159,16 +159,16 @@ STATIC mp_obj_t pb_type_Control_pid(size_t n_args, const mp_obj_t *pos_args, mp_
 STATIC MP_DEFINE_CONST_FUN_OBJ_KW(pb_type_Control_pid_obj, 1, pb_type_Control_pid);

 // pybricks._common.Control.target_tolerances


need to change comment to match rename

dlech · 2023-05-18T16:43:49Z

pybricks/common/pb_type_control.c

@@ -292,8 +261,7 @@ STATIC const pb_attr_dict_entry_t pb_type_Control_attr_dict[] = {
 STATIC const mp_rom_map_elem_t pb_type_Control_locals_dict_table[] = {
    { MP_ROM_QSTR(MP_QSTR_limits), MP_ROM_PTR(&pb_type_Control_limits_obj) },
    { MP_ROM_QSTR(MP_QSTR_pid), MP_ROM_PTR(&pb_type_Control_pid_obj) },
-    { MP_ROM_QSTR(MP_QSTR_target_tolerances), MP_ROM_PTR(&pb_type_Control_target_tolerances_obj) },
-    { MP_ROM_QSTR(MP_QSTR_stall_tolerances), MP_ROM_PTR(&pb_type_Control_stall_tolerances_obj) },
+    { MP_ROM_QSTR(MP_QSTR_tolerances), MP_ROM_PTR(&pb_type_Control_tolerances_obj) },


is this a breaking change?

dlech · 2023-05-18T16:46:49Z

pybricks/common/pb_type_motor.c

    }

-    if (ex != MP_OBJ_NULL) {
-        nlr_raise(ex);
+    // Get torque limit as percentage of max torque.


Why not use SI units for torque?

Then you'd need to look up the torque for each motor. I'm torn about this one --- a percentage is very convenient for this use case as it is only a temporary setting (and it is a percentage of something well defined).

dlech · 2023-05-18T16:48:58Z

pybricks/common/pb_type_motor.c

+    // Start moving.
+    pb_assert(pbio_servo_run_until_stalled(self->srv, speed, torque_limit_pct, then));
+
+    // Handle completion by awaiting or blocking.


Suggested change

// Handle completion by awaiting or blocking.

// Handle completion returning awaitable or blocking depending on run loop state.

dlech · 2023-05-18T17:09:01Z

pybricks/common/pb_type_speaker.c

-
-    return mp_const_none;
+    self->notes_generator = mp_getiter(notes_in, NULL);
+    self->note_duration = 4 * 60 * 1000 / pb_obj_get_int(tempo_in);


Deleted comment is helpful to explain why we are using these specific numbers.

dlech · 2023-05-18T17:26:37Z

pybricks/tools/pb_module_tools.c

+
+    // Require that duration is nonnegative small int. This makes it cheaper to
+    // test completion state in iteration loop.
+    time = pbio_int_math_bind(time, 0, INT32_MAX >> 2);


Suggested change

time = pbio_int_math_bind(time, 0, INT32_MAX >> 2);

time = pbio_int_math_bind(time, 0, MP_SMALL_INT_MAX);

dlech · 2023-05-18T17:32:26Z

pybricks/tools/pb_module_tools.c

-    { MP_ROM_QSTR(MP_QSTR___name__),    MP_ROM_QSTR(MP_QSTR_tools)      },
-    { MP_ROM_QSTR(MP_QSTR_wait),        MP_ROM_PTR(&tools_wait_obj)     },
-    { MP_ROM_QSTR(MP_QSTR_StopWatch),   MP_ROM_PTR(&pb_type_StopWatch)  },
+    { MP_ROM_QSTR(MP_QSTR___name__),    MP_ROM_QSTR(MP_QSTR_tools)                    },


I think it is less noise in the long run if we don't try to align the braces.

Suggested change

{ MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_tools) },

{ MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_tools) },

dlech · 2023-05-18T17:42:13Z

pybricks/tools/pb_type_awaitable.c

+        return;
+    }
+
+    mp_obj_list_t *awaitables = MP_OBJ_TO_PTR(awaitables_in);


Is this safe without a type check?

dlech · 2023-05-18T17:51:03Z

pybricks/tools/pb_type_awaitable.c

+
+    // Outside run loop, block until the operation is complete.
+    while (test_completion_func && !test_completion_func(obj, start_time)) {
+        mp_hal_delay_ms(1);


🤔 On ev3dev (at least with the current implementation) we need a delay here. But in general, MICROPY_EVENT_POLL_HOOK would be more appropriate here.

laurensvalk · 2023-05-18T18:36:39Z

This is looking really good.

Good to know. That is encouraging for taking this further 😄

I think the most difficult problem remaining is solving cancellation/closing that takes some time. In Python, this is solved using async with, but I don't the we want to have to write that all of the time.

Indeed. The direction I am currently heading in is ensuring cancellation can be instant. This seems to be possible so far for motors and I think sensors after the revision I'm currently working on.

In areas where it is harder (maybe like Bluetooth), we could consider being more strict about not allowing multiple uses on parallel "tasks" and raising an exception instead.

This function can be called from the pure-Python task module to inform blocking methods that they must return an awaitable generator.

This adds a new MotorWait object that is generator-like (send() and throw() are omitted since we shouldn't need them) to provide a non- blocking way to wait for completion of a motor operation.

This reworks the blocking case of run_until_stalled to just use the generator approach as well. This reduces code duplication. This would get even simpler if we didn't have handle the exception here but restored the voltage on cancellation in general.

This prepares for refactoring to reduce allocation of generator objects.

This only allocates a new generator if all previous generators are in use. In a "good" program with no conflicting motor tasks, this means nothing is allocated outside the motor object at all.

This prepares for adding additional control objectives as flags.

This implements running until stalled.

This will be used to replace the module-level wrapper around running forever and awaiting a stall condition. This simplifies those routines because it can just wait for completion just like any other run_* method. It is also useful to have this functionality (back) in pbio so that non-MicroPython implementation can still use this.

Stopping on stall and running until stall are different things.

This uses the new pbio implementation. It also renames the duty_limit kwarg but allows the original name for backwards compatibility.

In many cases, the await part is the same across methods and only the return value differs. This generalizes the awaitable by splitting the completion test and return value. This will be particularly useful for sensors later on. All methods await mode switches in the same way, but they have different return values.

With the return value available separately, we can get it directly instead of through iterating. This reinstates behavior close to the way it was before introducing async, in an effort not to reduce performance in the normal use case.

No longer needed now that run_until_stalled is handled in pbio.

These configs were only used to group function parameters. Since they will not be constant generally (each method may have its own return value getter), it is better to just keep the lengthy function args. As done in the drivebase and motor, they can still have a common function that populates the awaitable to avoid code repetition, which keeps code size about the same.

This broke while refactoring the awaitable code previously.

…n linked list. This avoids issues with garbage collected items of the linked list. It also makes the code easier to follow by using standard MicroPython tools.

This allows us to await processes without going through an explicit MicroPython object. This doesn't make a difference for the awaitables implemented so far, but this will be useful for Bluetooth and sensor awaitables. For example, for pupdevices the void *object can be the *iodev so that the same awaitable type can be used for all sensors, even though each of the sensor_obj_t are all slightly different.

This generalizes it to awaitable options instead of options strictly about cancellation. No code functionality is changed in this commit.

Instead of start time. This allows for implementing a timeout without increasing the allocated size of the awaitable. It also means we can correctly store the time for a wait instead of hacking the duration into a pointer type.

Prepares for handling via async.

We want to call this from most class inits.

This is an alternate way to use temporary limits in pbio (no nlr buf) but without a breaking change in the definition of duty limit or stall. See pybricks/support#1069

This lets us assign a keyboard shortcut in user settings to build it quickly.

This reverts commit 2e19eef90cf7590676d65f0e37a91b7bbb867d52.

Also rename so it doesn't mask the data of the process thread.

This does not need the object itself. Also fix enum names. It will also be used for operations other than cancel, so rename it.

Until we work out clever ways of cancellation or queueing, we can prevent certain resources from being used on more than one task at all.

The initialization code ran only on boot, which could cause it to get in a bad state the second time.

laurensvalk · 2023-05-26T14:32:25Z

This has been merged to get early testing in and make some quick progress.

Now we'll be iteratively testing and improving it, and optimizing for code size.

laurensvalk force-pushed the multitasking branch 3 times, most recently from a71130c to 436738d Compare May 8, 2023 10:09

laurensvalk mentioned this pull request May 12, 2023

[Feature] Change definition of stalled pybricks/support#1069

Open

laurensvalk force-pushed the multitasking branch 2 times, most recently from ba9e9f4 to c881dc2 Compare May 12, 2023 07:49

dlech reviewed May 18, 2023

View reviewed changes

laurensvalk mentioned this pull request May 19, 2023

Refactor src/uartdev for async and code size #169

Merged

laurensvalk force-pushed the multitasking branch 2 times, most recently from a81e39a to ffb651a Compare May 22, 2023 14:03

laurensvalk and others added 6 commits May 26, 2023 11:52

pybricks.tools: Add _set_run_loop_active().

541f349

This function can be called from the pure-Python task module to inform blocking methods that they must return an awaitable generator.

pybricks.common.Motor: Return generator object in run loop.

49f453e

This adds a new MotorWait object that is generator-like (send() and throw() are omitted since we shouldn't need them) to provide a non- blocking way to wait for completion of a motor operation.

pybricks.common.Motor: Async run_until_stalled.

8981245

pybricks.common.Motor: Move pb_type_MotorWait_obj_t.

02fe7fc

This prepares for refactoring to reduce allocation of generator objects.

pybricks.common.Motor: Recycle free generators.

ec73534

This only allocates a new generator if all previous generators are in use. In a "good" program with no conflicting motor tasks, this means nothing is allocated outside the motor object at all.

laurensvalk added 23 commits May 26, 2023 12:39

pbio/control: Generalize control type.

11ce5b7

This prepares for adding additional control objectives as flags.

pbio/control: Add stalling as control objective.

80b8f83

This implements running until stalled.

pbio/control: Add stalling as completion option.

d53c300

Stopping on stall and running until stall are different things.

pybricks.common.Motor: Update run_until_stall.

236e21b

This uses the new pbio implementation. It also renames the duty_limit kwarg but allows the original name for backwards compatibility.

pybricks.common.Motor: Drop temporary stall states.

f961d55

No longer needed now that run_until_stalled is handled in pbio.

pybricks/tools/pb_type_awaitable: Fix allocation of first awaitable.

692df86

This broke while refactoring the awaitable code previously.

pybricks/tools/pb_type_awaitable: Fix cancellation of linked objects.

b758ac1

pybricks/tools/pb_type_awaitable: Use mp_obj_list_t instead of our ow…

cd7304a

…n linked list. This avoids issues with garbage collected items of the linked list. It also makes the code easier to follow by using standard MicroPython tools.

pybricks/tools/pb_type_awaitable: Refactor options.

9c910ea

This generalizes it to awaitable options instead of options strictly about cancellation. No code functionality is changed in this commit.

pybricks/tools/pb_type_awaitable: Add force block option.

54db661

pybricks/tools/pb_type_awaitable: Use end time.

c62b0a9

Instead of start time. This allows for implementing a timeout without increasing the allocated size of the awaitable. It also means we can correctly store the time for a wait instead of hacking the duration into a pointer type.

pybricks.common.ColorLight: Callbacks return objects.

1f7e87f

Prepares for handling via async.

pybricks/tools/pb_type_awaitable: Share blocking assert.

7e3a73f

We want to call this from most class inits.

pybricks.common.Motor: Restore duty_limit behavior.

019ed1e

This is an alternate way to use temporary limits in pbio (no nlr buf) but without a breaking change in the definition of duty limit or stall. See pybricks/support#1069

.vscode/tasks: Add build task for Move Hub.

6f099d2

This lets us assign a keyboard shortcut in user settings to build it quickly.

pybricks/tools/pb_type_awaitable: Revert generalizing object type.

65cbbec

This reverts commit 2e19eef90cf7590676d65f0e37a91b7bbb867d52.

pbio/uartdev: Fix port_data not being static.

54408e9

Also rename so it doesn't mask the data of the process thread.

pybricks/pupdevices: Make sensors async-ready.

e8651fa

laurensvalk force-pushed the multitasking branch from ffb651a to e8651fa Compare May 26, 2023 10:41

laurensvalk added 3 commits May 26, 2023 13:41

pybricks/tools/awaitable: Refactor cancel_all.

3cf50b3

This does not need the object itself. Also fix enum names. It will also be used for operations other than cancel, so rename it.

pybricks/tools/awaitable: Add raise on busy option.

3afe954

Until we work out clever ways of cancellation or queueing, we can prevent certain resources from being used on more than one task at all.

pybricks.common.Speaker: Fix awaitable.

4b939b0

The initialization code ran only on boot, which could cause it to get in a bad state the second time.

laurensvalk merged commit 3dbb221 into master May 26, 2023

dlech deleted the multitasking branch May 26, 2023 14:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multitasking #166

Multitasking #166

laurensvalk commented May 5, 2023 •

edited

Loading

coveralls commented May 5, 2023 •

edited

Loading

dlech commented May 5, 2023 •

edited

Loading

laurensvalk commented May 6, 2023

dlech commented May 6, 2023

laurensvalk commented May 6, 2023 •

edited

Loading

laurensvalk commented May 8, 2023 •

edited

Loading

github-actions bot commented May 8, 2023 •

edited

Loading

dlech commented May 8, 2023

dlech commented May 8, 2023

laurensvalk commented May 8, 2023

dlech commented May 8, 2023

laurensvalk commented May 9, 2023 •

edited

Loading

laurensvalk commented May 12, 2023

dlech left a comment

dlech May 18, 2023

dlech May 18, 2023

dlech May 18, 2023

dlech May 18, 2023

laurensvalk May 19, 2023

dlech May 18, 2023

dlech May 18, 2023

dlech May 18, 2023

dlech May 18, 2023

dlech May 18, 2023

dlech May 18, 2023

laurensvalk commented May 18, 2023

laurensvalk commented May 26, 2023

		@@ -159,16 +159,16 @@ STATIC mp_obj_t pb_type_Control_pid(size_t n_args, const mp_obj_t *pos_args, mp_
		STATIC MP_DEFINE_CONST_FUN_OBJ_KW(pb_type_Control_pid_obj, 1, pb_type_Control_pid);

		// pybricks._common.Control.target_tolerances

	// Handle completion by awaiting or blocking.
	// Handle completion returning awaitable or blocking depending on run loop state.

	time = pbio_int_math_bind(time, 0, INT32_MAX >> 2);
	time = pbio_int_math_bind(time, 0, MP_SMALL_INT_MAX);

	{ MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_tools) },
	{ MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_tools) },

Multitasking #166

Multitasking #166

Conversation

laurensvalk commented May 5, 2023 • edited Loading

coveralls commented May 5, 2023 • edited Loading

dlech commented May 5, 2023 • edited Loading

laurensvalk commented May 6, 2023

dlech commented May 6, 2023

laurensvalk commented May 6, 2023 • edited Loading

laurensvalk commented May 8, 2023 • edited Loading

github-actions bot commented May 8, 2023 • edited Loading

dlech commented May 8, 2023

dlech commented May 8, 2023

laurensvalk commented May 8, 2023

dlech commented May 8, 2023

laurensvalk commented May 9, 2023 • edited Loading

laurensvalk commented May 12, 2023

dlech left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laurensvalk commented May 18, 2023

laurensvalk commented May 26, 2023

laurensvalk commented May 5, 2023 •

edited

Loading

coveralls commented May 5, 2023 •

edited

Loading

dlech commented May 5, 2023 •

edited

Loading

laurensvalk commented May 6, 2023 •

edited

Loading

laurensvalk commented May 8, 2023 •

edited

Loading

github-actions bot commented May 8, 2023 •

edited

Loading

laurensvalk commented May 9, 2023 •

edited

Loading