Skip to content

Commit

Permalink
pbio/light/animation: Fix lockup on restarting animation.
Browse files Browse the repository at this point in the history
If it was restarted just as a new frame was being loaded,
the animation would get stuck handling timer events forever.

Fixes pybricks/support#1295
  • Loading branch information
laurensvalk committed Oct 4, 2024
1 parent a047055 commit 4ff6e7b
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 10 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
loops ([support#1668]).
- Fixed program restarting if the stop button was held to end the program
without an exception ([support#1863]).
- Fixed program lockup when restarting a hub light or light matrix animation
at exact multiples of its animation interval ([support#1295]).

[support#1295]: https://github.com/pybricks/support/issues/1295
[support#1623]: https://github.com/pybricks/support/issues/1623
[support#1661]: https://github.com/pybricks/support/issues/1661
[support#1668]: https://github.com/pybricks/support/issues/1668
Expand Down
12 changes: 6 additions & 6 deletions lib/pbio/src/light/animation.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ void pbio_light_animation_start(pbio_light_animation_t *animation) {
pbio_light_animation_list_head = animation;

process_start(&pbio_light_animation_process);
// HACK: init timer since we don't call etimer_set()
timer_set(&animation->timer.timer, 0);
// fake a timer event to load the first cell and start the timer
process_post_synch(&pbio_light_animation_process, PROCESS_EVENT_TIMER, &animation->timer);
// Fake a timer event to load the first cell.
PROCESS_CONTEXT_BEGIN(&pbio_light_animation_process);
etimer_set(&animation->timer, 0);
PROCESS_CONTEXT_END(&pbio_light_animation_process);

assert(animation->next_animation != PBIO_LIGHT_ANIMATION_STOPPED);
}
Expand Down Expand Up @@ -114,11 +114,11 @@ PROCESS_THREAD(pbio_light_animation_process, ev, data) {
PROCESS_BEGIN();

for (;;) {
PROCESS_WAIT_EVENT_UNTIL(ev == PROCESS_EVENT_TIMER);
PROCESS_WAIT_EVENT_UNTIL(ev == PROCESS_EVENT_TIMER && etimer_expired(data));
struct etimer *timer = data;
pbio_light_animation_t *animation = PBIO_CONTAINER_OF(timer, pbio_light_animation_t, timer);
clock_time_t interval = animation->next(animation);
if (pbio_light_animation_is_started(animation)) {
clock_time_t interval = animation->next(animation);
etimer_reset_with_new_interval(&animation->timer, interval);
}
}
Expand Down
4 changes: 3 additions & 1 deletion lib/pbio/test/src/test_animation.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@ static PT_THREAD(test_light_animation(struct pt *pt)) {
tt_want(!process_is_running(&pbio_light_animation_process));
tt_want(!pbio_light_animation_is_started(&test_animation));

// starting animation should start process and call next() once synchonously
// starting animation should start process and set a timer at 0ms to call
// next() after handling pending events
pbio_light_animation_start(&test_animation);
tt_want(pbio_light_animation_is_started(&test_animation));
tt_want(process_is_running(&pbio_light_animation_process));
pbio_handle_pending_events();
tt_want_uint_op(test_animation_set_hsv_call_count, ==, 1);

// next() should not be called again until after a delay
Expand Down
6 changes: 4 additions & 2 deletions lib/pbio/test/src/test_color_light.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ static PT_THREAD(test_color_light(struct pt *pt)) {
// reset call count for next series of tests
test_light_set_hsv_call_count = 0;

// starting animation should call set_hsv() synchonously
// starting animation should call set_hsv() after handling pending events
static const pbio_color_hsv_t hsv = { .h = PBIO_COLOR_HUE_BLUE, .s = 100, .v = 100 };
pbio_color_light_start_blink_animation(&test_light, &hsv, test_blink);
pbio_handle_pending_events();
tt_want_uint_op(test_light_set_hsv_call_count, ==, 1);
// even blink cells turns the light on
tt_want_uint_op(test_light_set_hsv_last_hue, ==, PBIO_COLOR_HUE_BLUE);
Expand All @@ -92,8 +93,9 @@ static PT_THREAD(test_color_light(struct pt *pt)) {
// reset call count for next series of tests
test_light_set_hsv_call_count = 0;

// starting animation should call set_hsv() synchonously
// starting animation should call set_hsv() after handling pending events
pbio_color_light_start_animation(&test_light, TEST_ANIMATION_TIME, test_animation);
pbio_handle_pending_events();
tt_want_uint_op(test_light_set_hsv_call_count, ==, 1);
tt_want_uint_op(test_light_set_hsv_last_hue, ==, PBIO_COLOR_HUE_CYAN);

Expand Down
4 changes: 3 additions & 1 deletion lib/pbio/test/src/test_light_matrix.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,11 @@ static PT_THREAD(test_light_matrix(struct pt *pt)) {
IMAGE_DATA(1, 2, 3, 4, 5, 6, 7, 8, 9)), ==, PBIO_SUCCESS);
tt_want_light_matrix_data(1, 2, 3, 4, 5, 6, 7, 8, 9);

// starting animation should call set_pixel() synchonously with the first cell data
// starting animation should schedule timer event at 0 ms to call
// set_pixel() after handling pending events.
test_light_matrix_reset();
pbio_light_matrix_start_animation(&test_light_matrix, test_animation, 2, INTERVAL);
pbio_handle_pending_events();
tt_want_light_matrix_data(1, 2, 3, 4, 5, 6, 7, 8, 9);

// set_pixel() should not be called again until after a delay and it should
Expand Down
6 changes: 6 additions & 0 deletions lib/pbio/test/test-pbio.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <pbio/button.h>
#include <pbio/int_math.h>
#include <pbio/main.h>

// Use this macro to define tests that _don't_ require a Contiki event loop
#define PBIO_TEST(name) \
Expand Down Expand Up @@ -57,6 +58,11 @@ void pbio_test_counter_set_abs_angle(int32_t millidegrees);
PT_YIELD(pt); \
}

static inline void pbio_handle_pending_events(void) {
while (pbio_do_one_event()) {
}
}

#define pbio_test_int_is_close(value, target, tolerance) (pbio_int_math_abs((value) - (target)) <= (tolerance))

#endif // _TEST_PBIO_H_

0 comments on commit 4ff6e7b

Please sign in to comment.