From de76f73d34a63fa34c520125f226e0c159ce0b10 Mon Sep 17 00:00:00 2001 From: Martin Dybdal Date: Fri, 24 May 2019 13:19:26 +0200 Subject: [PATCH] esp32/machine_timer: Reuse Timer handles, deallocate only on soft-reset. The patch solves the problem where multiple Timer objects (e.g. multiple Timer(0) instances) could initialise multiple handles to the same internal timer. The list of timers is now maintained not for "active" timers (where init is called), but for all timers created. The timers are only removed from the list of timers on soft-reset (machine_timer_deinit_all). Fixes #4078. --- ports/esp32/machine_timer.c | 40 ++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/ports/esp32/machine_timer.c b/ports/esp32/machine_timer.c index 081a46b9c..c941d943b 100644 --- a/ports/esp32/machine_timer.c +++ b/ports/esp32/machine_timer.c @@ -73,8 +73,13 @@ STATIC esp_err_t check_esp_err(esp_err_t code) { } void machine_timer_deinit_all(void) { - while (MP_STATE_PORT(machine_timer_obj_head) != NULL) { - machine_timer_disable(MP_STATE_PORT(machine_timer_obj_head)); + // Disable, deallocate and remove all timers from list + machine_timer_obj_t **t = &MP_STATE_PORT(machine_timer_obj_head); + while (*t != NULL) { + machine_timer_disable(*t); + machine_timer_obj_t *next = (*t)->next; + m_del_obj(machine_timer_obj_t, *t); + *t = next; } } @@ -93,12 +98,24 @@ STATIC void machine_timer_print(const mp_print_t *print, mp_obj_t self_in, mp_pr STATIC mp_obj_t machine_timer_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) { mp_arg_check_num(n_args, n_kw, 1, 1, false); + mp_uint_t group = (mp_obj_get_int(args[0]) >> 1) & 1; + mp_uint_t index = mp_obj_get_int(args[0]) & 1; + + // Check whether the timer is already initialized, if so return it + for (machine_timer_obj_t *t = MP_STATE_PORT(machine_timer_obj_head); t; t = t->next) { + if (t->group == group && t->index == index) { + return t; + } + } + machine_timer_obj_t *self = m_new_obj(machine_timer_obj_t); self->base.type = &machine_timer_type; + self->group = group; + self->index = index; - self->group = (mp_obj_get_int(args[0]) >> 1) & 1; - self->index = mp_obj_get_int(args[0]) & 1; - self->next = NULL; + // Add the timer to the linked-list of timers + self->next = MP_STATE_PORT(machine_timer_obj_head); + MP_STATE_PORT(machine_timer_obj_head) = self; return self; } @@ -110,13 +127,8 @@ STATIC void machine_timer_disable(machine_timer_obj_t *self) { self->handle = NULL; } - // Remove the timer from the linked-list of active timers - for (machine_timer_obj_t **t = &MP_STATE_PORT(machine_timer_obj_head); *t; t = &(*t)->next) { - if (*t == self) { - *t = (*t)->next; - break; - } - } + // We let the disabled timer stay in the list, as it might be + // referenced elsewhere } STATIC void machine_timer_isr(void *self_in) { @@ -150,10 +162,6 @@ STATIC void machine_timer_enable(machine_timer_obj_t *self) { check_esp_err(timer_enable_intr(self->group, self->index)); check_esp_err(timer_isr_register(self->group, self->index, machine_timer_isr, (void*)self, TIMER_FLAGS, &self->handle)); check_esp_err(timer_start(self->group, self->index)); - - // Add the timer to the linked-list of active timers - self->next = MP_STATE_PORT(machine_timer_obj_head); - MP_STATE_PORT(machine_timer_obj_head) = self; } STATIC mp_obj_t machine_timer_init_helper(machine_timer_obj_t *self, mp_uint_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {