Skip to content

Commit

Permalink
pybricks/tools/pb_type_awaitable: Use mp_obj_list_t instead of our ow…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
laurensvalk committed May 12, 2023
1 parent a095e0d commit c881dc2
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 75 deletions.
2 changes: 1 addition & 1 deletion pybricks/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ struct _common_Motor_obj_t {
mp_obj_t logger;
#endif
pbio_port_id_t port;
pb_type_awaitable_obj_t *first_awaitable;
mp_obj_t awaitables;
};

extern const mp_obj_type_t pb_type_Motor;
Expand Down
16 changes: 9 additions & 7 deletions pybricks/common/pb_type_motor.c
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ STATIC mp_obj_t common_Motor_make_new(const mp_obj_type_t *type, size_t n_args,
self->logger = common_Logger_obj_make_new(&self->srv->log, PBIO_SERVO_LOGGER_NUM_COLS);
#endif

self->first_awaitable = NULL;
// List of awaitables associated with this motor. By keeping track,
// we can cancel them as needed when a new movement is started.
self->awaitables = mp_obj_new_list(0, NULL);

return MP_OBJ_FROM_PTR(self);
}
Expand All @@ -148,7 +150,7 @@ STATIC void common_Motor_cancel(mp_obj_t self_in) {
STATIC mp_obj_t await_or_wait(common_Motor_obj_t *self) {
return pb_type_awaitable_await_or_wait(
MP_OBJ_FROM_PTR(self),
&self->first_awaitable,
self->awaitables,
common_Motor_test_completion,
pb_type_awaitable_return_none,
common_Motor_cancel,
Expand Down Expand Up @@ -179,7 +181,7 @@ STATIC mp_obj_t common_Motor_reset_angle(size_t n_args, const mp_obj_t *pos_args

// Set the new angle
pb_assert(pbio_servo_reset_angle(self->srv, reset_angle, reset_to_abs));
pb_type_awaitable_cancel_all(pos_args[0], self->first_awaitable, PB_TYPE_AWAITABLE_CANCEL_AWAITABLE);
pb_type_awaitable_cancel_all(MP_OBJ_FROM_PTR(self), self->awaitables, PB_TYPE_AWAITABLE_CANCEL_AWAITABLE);
return mp_const_none;
}
STATIC MP_DEFINE_CONST_FUN_OBJ_KW(common_Motor_reset_angle_obj, 1, common_Motor_reset_angle);
Expand All @@ -204,7 +206,7 @@ STATIC mp_obj_t common_Motor_run(size_t n_args, const mp_obj_t *pos_args, mp_map

mp_int_t speed = pb_obj_get_int(speed_in);
pb_assert(pbio_servo_run_forever(self->srv, speed));
pb_type_awaitable_cancel_all(pos_args[0], self->first_awaitable, PB_TYPE_AWAITABLE_CANCEL_AWAITABLE);
pb_type_awaitable_cancel_all(MP_OBJ_FROM_PTR(self), self->awaitables, PB_TYPE_AWAITABLE_CANCEL_AWAITABLE);
return mp_const_none;
}
STATIC MP_DEFINE_CONST_FUN_OBJ_KW(common_Motor_run_obj, 1, common_Motor_run);
Expand All @@ -213,7 +215,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_KW(common_Motor_run_obj, 1, common_Motor_run);
STATIC mp_obj_t common_Motor_hold(mp_obj_t self_in) {
common_Motor_obj_t *self = MP_OBJ_TO_PTR(self_in);
pb_assert(pbio_servo_stop(self->srv, PBIO_CONTROL_ON_COMPLETION_HOLD));
pb_type_awaitable_cancel_all(self_in, self->first_awaitable, PB_TYPE_AWAITABLE_CANCEL_AWAITABLE);
pb_type_awaitable_cancel_all(self_in, self->awaitables, PB_TYPE_AWAITABLE_CANCEL_AWAITABLE);
return mp_const_none;
}
MP_DEFINE_CONST_FUN_OBJ_1(common_Motor_hold_obj, common_Motor_hold);
Expand Down Expand Up @@ -284,7 +286,7 @@ STATIC mp_obj_t common_Motor_run_until_stalled(size_t n_args, const mp_obj_t *po
// Handle completion by awaiting or blocking.
return pb_type_awaitable_await_or_wait(
MP_OBJ_FROM_PTR(self),
&self->first_awaitable,
self->awaitables,
common_Motor_test_completion,
common_Motor_stall_return_value,
common_Motor_cancel,
Expand Down Expand Up @@ -350,7 +352,7 @@ STATIC mp_obj_t common_Motor_track_target(size_t n_args, const mp_obj_t *pos_arg

mp_int_t target_angle = pb_obj_get_int(target_angle_in);
pb_assert(pbio_servo_track_target(self->srv, target_angle));
pb_type_awaitable_cancel_all(pos_args[0], self->first_awaitable, PB_TYPE_AWAITABLE_CANCEL_AWAITABLE);
pb_type_awaitable_cancel_all(MP_OBJ_FROM_PTR(self), self->awaitables, PB_TYPE_AWAITABLE_CANCEL_AWAITABLE);
return mp_const_none;
}
STATIC MP_DEFINE_CONST_FUN_OBJ_KW(common_Motor_track_target_obj, 1, common_Motor_track_target);
Expand Down
11 changes: 7 additions & 4 deletions pybricks/common/pb_type_speaker.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ typedef struct {
uint32_t note_duration;
uint32_t beep_end_time;
uint32_t release_end_time;
pb_type_awaitable_obj_t *first_awaitable;
mp_obj_t awaitables;

// volume in 0..100 range
uint8_t volume;
Expand Down Expand Up @@ -115,7 +115,10 @@ STATIC mp_obj_t pb_type_Speaker_make_new(const mp_obj_type_t *type, size_t n_arg
if (!self->initialized) {
self->base.type = &pb_type_Speaker;
self->initialized = true;
self->first_awaitable = NULL;

// List of awaitables associated with speaker. By keeping track,
// we can cancel them as needed when a new sound is started.
self->awaitables = mp_obj_new_list(0, NULL);
}

// REVISIT: If a user creates two Speaker instances, this will reset the volume settings for both.
Expand Down Expand Up @@ -164,7 +167,7 @@ STATIC mp_obj_t pb_type_Speaker_beep(size_t n_args, const mp_obj_t *pos_args, mp

return pb_type_awaitable_await_or_wait(
MP_OBJ_FROM_PTR(self),
&self->first_awaitable,
self->awaitables,
pb_type_Speaker_beep_test_completion,
pb_type_awaitable_return_none,
pb_type_Speaker_cancel,
Expand Down Expand Up @@ -379,7 +382,7 @@ STATIC mp_obj_t pb_type_Speaker_play_notes(size_t n_args, const mp_obj_t *pos_ar
self->release_end_time = self->beep_end_time;
return pb_type_awaitable_await_or_wait(
MP_OBJ_FROM_PTR(self),
&self->first_awaitable,
self->awaitables,
pb_type_Speaker_notes_test_completion,
pb_type_awaitable_return_none,
pb_type_Speaker_cancel,
Expand Down
12 changes: 7 additions & 5 deletions pybricks/robotics/pb_type_drivebase.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ struct _pb_type_DriveBase_obj_t {
mp_obj_t heading_control;
mp_obj_t distance_control;
#endif
pb_type_awaitable_obj_t *first_awaitable;
mp_obj_t awaitables;
};

// pybricks.robotics.DriveBase.reset
Expand Down Expand Up @@ -85,7 +85,9 @@ STATIC mp_obj_t pb_type_DriveBase_make_new(const mp_obj_type_t *type, size_t n_a
// Reset drivebase state
pb_type_DriveBase_reset(MP_OBJ_FROM_PTR(self));

self->first_awaitable = NULL;
// List of awaitables associated with this drivebase. By keeping track,
// we can cancel them as needed when a new movement is started.
self->awaitables = mp_obj_new_list(0, NULL);

return MP_OBJ_FROM_PTR(self);
}
Expand All @@ -112,7 +114,7 @@ STATIC void pb_type_DriveBase_cancel(mp_obj_t self_in) {
STATIC mp_obj_t await_or_wait(pb_type_DriveBase_obj_t *self) {
return pb_type_awaitable_await_or_wait(
MP_OBJ_FROM_PTR(self),
&self->first_awaitable,
self->awaitables,
pb_type_DriveBase_test_completion,
pb_type_awaitable_return_none,
pb_type_DriveBase_cancel,
Expand Down Expand Up @@ -200,7 +202,7 @@ STATIC mp_obj_t pb_type_DriveBase_drive(size_t n_args, const mp_obj_t *pos_args,
mp_int_t turn_rate = pb_obj_get_int(turn_rate_in);

// Cancel awaitables but not hardware. Drive forever will handle this.
pb_type_awaitable_cancel_all(pos_args[0], self->first_awaitable, PB_TYPE_AWAITABLE_CANCEL_AWAITABLE);
pb_type_awaitable_cancel_all(MP_OBJ_FROM_PTR(self), self->awaitables, PB_TYPE_AWAITABLE_CANCEL_AWAITABLE);

pb_assert(pbio_drivebase_drive_forever(self->db, speed, turn_rate));
return mp_const_none;
Expand All @@ -212,7 +214,7 @@ STATIC mp_obj_t pb_type_DriveBase_stop(mp_obj_t self_in) {

// Cancel awaitables.
pb_type_DriveBase_obj_t *self = MP_OBJ_TO_PTR(self_in);
pb_type_awaitable_cancel_all(self_in, self->first_awaitable, PB_TYPE_AWAITABLE_CANCEL_AWAITABLE);
pb_type_awaitable_cancel_all(self_in, self->awaitables, PB_TYPE_AWAITABLE_CANCEL_AWAITABLE);

// Stop hardware.
pb_type_DriveBase_cancel(self_in);
Expand Down
40 changes: 21 additions & 19 deletions pybricks/tools/pb_module_tools.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,26 @@
#include <pybricks/util_mp/pb_obj_helper.h>
#include <pybricks/util_pb/pb_error.h>

// The awaitable for the wait() function has no object associated with
// it (unlike e.g. a motor), so we make a starting point here. This gets
// cleared when the module initializes.
STATIC pb_type_awaitable_obj_t *first_wait_awaitable = NULL;

// Global state of the run loop for async user programs. Gets set when run_task
// is called and cleared when it completes
STATIC bool run_loop_is_active;

bool pb_module_tools_run_loop_is_active() {
return run_loop_is_active;
}

// The awaitables for the wait() function have no object associated with
// it (unlike e.g. a motor), so we make a starting point here. These never
// have to cancel each other so shouldn't need to be in a list, but this lets
// us share the same code with other awaitables. It also minimizes allocation.
MP_REGISTER_ROOT_POINTER(mp_obj_t wait_awaitables);

// Reset global state when user program starts.
void pb_module_tools_init(void) {
MP_STATE_PORT(wait_awaitables) = mp_obj_new_list(0, NULL);
run_loop_is_active = false;
}

STATIC bool pb_module_tools_wait_test_completion(mp_obj_t obj, uint32_t start_time) {
// obj was validated to be small int, so we can do a cheap comparison here.
Expand All @@ -52,28 +68,14 @@ STATIC mp_obj_t pb_module_tools_wait(size_t n_args, const mp_obj_t *pos_args, mp

return pb_type_awaitable_await_or_wait(
MP_OBJ_NEW_SMALL_INT(time),
&first_wait_awaitable,
MP_STATE_PORT(wait_awaitables),
pb_module_tools_wait_test_completion,
pb_type_awaitable_return_none,
pb_type_awaitable_cancel_none,
PB_TYPE_AWAITABLE_CANCEL_NONE);
}
STATIC MP_DEFINE_CONST_FUN_OBJ_KW(pb_module_tools_wait_obj, 0, pb_module_tools_wait);

// Global state of the run loop for async user programs. Gets set when run_task
// is called and cleared when it completes
STATIC bool run_loop_is_active;

bool pb_module_tools_run_loop_is_active() {
return run_loop_is_active;
}

// Reset global state when user program starts.
void pb_module_tools_init(void) {
first_wait_awaitable = NULL;
run_loop_is_active = false;
}

STATIC mp_obj_t pb_module_tools_run_task(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
PB_PARSE_ARGS_FUNCTION(n_args, pos_args, kw_args,
PB_ARG_REQUIRED(task),
Expand Down
71 changes: 34 additions & 37 deletions pybricks/tools/pb_type_awaitable.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "py/mphal.h"
#include "py/mpstate.h"
#include "py/obj.h"
#include "py/runtime.h"

#include <pybricks/tools.h>
Expand Down Expand Up @@ -38,10 +39,6 @@ struct _pb_type_awaitable_obj_t {
* Called on cancellation.
*/
pb_type_awaitable_cancel_t cancel;
/**
* Linked list of awaitables.
*/
pb_type_awaitable_obj_t *next_awaitable;
};

// close() cancels the awaitable.
Expand Down Expand Up @@ -94,26 +91,31 @@ MP_DEFINE_CONST_OBJ_TYPE(pb_type_awaitable,
iter, pb_type_awaitable_iternext,
locals_dict, &pb_type_awaitable_locals_dict);

STATIC pb_type_awaitable_obj_t *pb_type_awaitable_get(pb_type_awaitable_obj_t *first_awaitable) {
/**
* Gets an awaitable object that is not in use, or makes a new one.
*
* @param [in] awaitables_in List of awaitables associated with @p obj.
*/
STATIC pb_type_awaitable_obj_t *pb_type_awaitable_get(mp_obj_t awaitables_in) {

// Find next available awaitable that exists and is not used.
pb_type_awaitable_obj_t *awaitable = first_awaitable;
while (awaitable->test_completion != AWAITABLE_FREE && awaitable->next_awaitable) {
awaitable = awaitable->next_awaitable;
}
mp_obj_list_t *awaitables = MP_OBJ_TO_PTR(awaitables_in);

// Loop stopped because available awaitable was found, so return it.
if (awaitable->test_completion == AWAITABLE_FREE) {
return awaitable;
}
for (size_t i = 0; i < awaitables->len; i++) {
pb_type_awaitable_obj_t *awaitable = MP_OBJ_TO_PTR(awaitables->items[i]);

// Otherwise no available awaitable was found, so allocate a new one
awaitable->next_awaitable = mp_obj_malloc(pb_type_awaitable_obj_t, &pb_type_awaitable);
// Return awaitable if it is not in use.
if (awaitable->test_completion == AWAITABLE_FREE) {
return awaitable;
}
}

// Initialize the new awaitable.
awaitable = awaitable->next_awaitable;
// Otherwise allocate a new one.
pb_type_awaitable_obj_t *awaitable = mp_obj_malloc(pb_type_awaitable_obj_t, &pb_type_awaitable);
awaitable->test_completion = AWAITABLE_FREE;
awaitable->next_awaitable = NULL;

// Add to list of awaitables.
mp_obj_list_append(awaitables_in, MP_OBJ_FROM_PTR(awaitable));

return awaitable;
}

Expand All @@ -135,18 +137,20 @@ STATIC bool pb_type_awaitable_completed(mp_obj_t self_in, uint32_t start_time) {
* also be called independently to cancel without starting a new awaitable.
*
* @param [in] obj The object whose method we want to wait for completion.
* @param [in] first_awaitable The first awaitable in the linked list of awaitables from @p obj.
* @param [in] awaitables_in List of awaitables associated with @p obj.
* @param [in] cancel_opt Whether to cancel linked awaitables, hardware, or both.
*/
void pb_type_awaitable_cancel_all(mp_obj_t obj, pb_type_awaitable_obj_t *first_awaitable, pb_type_awaitable_cancel_opt_t cancel_opt) {
void pb_type_awaitable_cancel_all(mp_obj_t obj, mp_obj_t awaitables_in, pb_type_awaitable_cancel_opt_t cancel_opt) {

// Exit if nothing to do.
if (!pb_module_tools_run_loop_is_active() || cancel_opt == PB_TYPE_AWAITABLE_CANCEL_NONE || !first_awaitable) {
if (!pb_module_tools_run_loop_is_active() || cancel_opt == PB_TYPE_AWAITABLE_CANCEL_NONE) {
return;
}

pb_type_awaitable_obj_t *awaitable = first_awaitable;
while (awaitable) {
mp_obj_list_t *awaitables = MP_OBJ_TO_PTR(awaitables_in);

for (size_t i = 0; i < awaitables->len; i++) {
pb_type_awaitable_obj_t *awaitable = MP_OBJ_TO_PTR(awaitables->items[i]);
// Only cancel awaitables that are in use.
if (awaitable->test_completion) {
// Cancel hardware operation if requested and available.
Expand All @@ -159,7 +163,6 @@ void pb_type_awaitable_cancel_all(mp_obj_t obj, pb_type_awaitable_obj_t *first_a
awaitable->test_completion = pb_type_awaitable_completed;
}
}
awaitable = awaitable->next_awaitable;
}
}

Expand All @@ -169,15 +172,15 @@ void pb_type_awaitable_cancel_all(mp_obj_t obj, pb_type_awaitable_obj_t *first_a
* Automatically cancels any previous awaitables associated with the object if requested.
*
* @param [in] obj The object whose method we want to wait for completion.
* @param [in] first_awaitable The first awaitable in the linked list of awaitables from @p obj.
* @param [in] awaitables_in List of awaitables associated with @p obj.
* @param [in] test_completion_func Function to test if the operation is complete.
* @param [in] return_value_func Function that gets the return value for the awaitable.
* @param [in] cancel_func Function to cancel the hardware operation.
* @param [in] cancel_opt Whether to cancel linked awaitables, hardware, or both.
*/
mp_obj_t pb_type_awaitable_await_or_wait(
mp_obj_t obj,
pb_type_awaitable_obj_t **first_awaitable,
mp_obj_t awaitables_in,
pb_type_awaitable_test_completion_t test_completion_func,
pb_type_awaitable_return_t return_value_func,
pb_type_awaitable_cancel_t cancel_func,
Expand All @@ -187,18 +190,12 @@ mp_obj_t pb_type_awaitable_await_or_wait(

// Within run loop, return the generator that user program will iterate.
if (pb_module_tools_run_loop_is_active()) {
// First cancel linked awaitables if requested.
pb_type_awaitable_cancel_all(obj, *first_awaitable, cancel_opt);

// If the first awaitable was not yet created, do so now.
if (!*first_awaitable) {
*first_awaitable = mp_obj_malloc(pb_type_awaitable_obj_t, &pb_type_awaitable);
(*first_awaitable)->test_completion = AWAITABLE_FREE;
(*first_awaitable)->next_awaitable = NULL;
}
// First cancel linked awaitables if requested.
pb_type_awaitable_cancel_all(obj, awaitables_in, cancel_opt);

// Gets existing awaitable or creates a new one.
pb_type_awaitable_obj_t *awaitable = pb_type_awaitable_get(*first_awaitable);
// Gets free existing awaitable or creates a new one.
pb_type_awaitable_obj_t *awaitable = pb_type_awaitable_get(awaitables_in);

// Initialize awaitable.
awaitable->obj = obj;
Expand Down
4 changes: 2 additions & 2 deletions pybricks/tools/pb_type_awaitable.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,11 @@ typedef void (*pb_type_awaitable_cancel_t)(mp_obj_t obj);

#define pb_type_awaitable_cancel_none (NULL)

void pb_type_awaitable_cancel_all(mp_obj_t obj, pb_type_awaitable_obj_t *first_awaitable, pb_type_awaitable_cancel_opt_t cancel_opt);
void pb_type_awaitable_cancel_all(mp_obj_t obj, mp_obj_t awaitables_in, pb_type_awaitable_cancel_opt_t cancel_opt);

mp_obj_t pb_type_awaitable_await_or_wait(
mp_obj_t obj,
pb_type_awaitable_obj_t **first_awaitable,
mp_obj_t awaitables_in,
pb_type_awaitable_test_completion_t test_completion_func,
pb_type_awaitable_return_t return_value_func,
pb_type_awaitable_cancel_t cancel_func,
Expand Down

0 comments on commit c881dc2

Please sign in to comment.