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

Silent failure of timer_isr_callback_add when using multiple timers (IDFGH-8155) #9651

Closed
3 tasks done
kristinpaget opened this issue Aug 26, 2022 · 2 comments
Closed
3 tasks done
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Bug bugs in IDF

Comments

@kristinpaget
Copy link

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v4.4.2

Operating System used.

Linux

How did you build your project?

Command line with idf.py

If you are using Windows, please specify command line type.

No response

Development Kit.

ESP32-WROOM custom board

Power Supply used.

USB

What is the expected behavior?

When using timer callbacks with multiple timers configured as LEVEL3 interrupts, the second call to timer_isr_callback_add() fails silently without producing an error and without enabling the second interrupt. If a call to timer_isr_callback_add() results in an interrupt not being enabled then I expect an error to be returned.

What is the actual behavior?

No error is returned by timer_isr_callback_add() leading high-level code to believe that the ISR has been successfully enabled when it has not.

Steps to reproduce.

Compile and run the following code example:

#include <stdio.h>
#include "driver/timer.h"
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "freertos/queue.h"
#include <esp_log.h>

#define INTR_FLAGS ESP_INTR_FLAG_LEVEL3

QueueHandle_t TimerQueue;

static bool IRAM_ATTR TimerCallback(void *Arg)
{
	uint32_t TimerNum = (uint32_t)Arg;
	BaseType_t high_task_awoken = pdFALSE;
	xQueueSendFromISR(TimerQueue, &TimerNum, &high_task_awoken);
	return high_task_awoken == pdTRUE; // return whether a task switch is needed
}

void app_main(void)
{	
	TimerQueue = xQueueCreate(10, sizeof(uint32_t));
	assert(TimerQueue);

	vTaskDelay(1000 / portTICK_PERIOD_MS);
	printf("Starting timers\n");

	timer_config_t Timer0Config = 
	{
		.divider = 8,
		.counter_dir = TIMER_COUNT_UP,
		.counter_en = TIMER_PAUSE,
		.alarm_en = TIMER_ALARM_EN,
		.auto_reload = true
	};
	ESP_ERROR_CHECK(timer_init(TIMER_GROUP_0, 0, &Timer0Config));
	ESP_ERROR_CHECK(timer_set_counter_value(TIMER_GROUP_0, 0, 0));
	ESP_ERROR_CHECK(timer_set_alarm_value(TIMER_GROUP_0, 0, 10000000));
	ESP_ERROR_CHECK(timer_enable_intr(TIMER_GROUP_0, 0));
	ESP_ERROR_CHECK(timer_isr_callback_add(TIMER_GROUP_0, 0, TimerCallback, (void *)0, INTR_FLAGS));
	ESP_ERROR_CHECK(timer_start(TIMER_GROUP_0, 0));

	timer_config_t Timer1Config = 
	{
		.divider = 8,
		.counter_dir = TIMER_COUNT_UP,
		.counter_en = TIMER_PAUSE,
		.alarm_en = TIMER_ALARM_EN,
		.auto_reload = true
	};
	ESP_ERROR_CHECK(timer_init(TIMER_GROUP_0, 1, &Timer1Config));
	ESP_ERROR_CHECK(timer_set_counter_value(TIMER_GROUP_0, 1, 0));
	ESP_ERROR_CHECK(timer_set_alarm_value(TIMER_GROUP_0, 1, 10000000));
	ESP_ERROR_CHECK(timer_enable_intr(TIMER_GROUP_0, 1));
	ESP_ERROR_CHECK(timer_isr_callback_add(TIMER_GROUP_0, 1, TimerCallback, (void *)1, INTR_FLAGS));
	ESP_ERROR_CHECK(timer_start(TIMER_GROUP_0, 1));

	while (1)
	{
		uint32_t TimerNumber;
		xQueueReceive(TimerQueue, &TimerNumber, portMAX_DELAY);
		ESP_LOGI("Timer", "Timer %u", TimerNumber);
	}	
}

The second call to timer_isr_callback_add fails silently without enabling the interrupt for the second timer.

If the #define INTR_FLAGS line at the top is modified to ESP_INTR_FLAG_LEVEL2 then the code succeeds and both timers work as expected.

Debug Logs.

For the failing Level3 interrupt:
V (1253) intr_alloc: esp_intr_alloc_intrstatus (cpu 0): checking args
V (1263) intr_alloc: esp_intr_alloc_intrstatus (cpu 0): Args okay. Resulting flags 0x8
D (1263) intr_alloc: Connected src 14 to int 23 (cpu 0)
V (1273) intr_alloc: esp_intr_alloc_intrstatus (cpu 0): checking args
V (1283) intr_alloc: esp_intr_alloc_intrstatus (cpu 0): Args okay. Resulting flags 0x8

For the successful case with a LEVEL2 interrupt:
V (1253) intr_alloc: esp_intr_alloc_intrstatus (cpu 0): checking args
V (1263) intr_alloc: esp_intr_alloc_intrstatus (cpu 0): Args okay. Resulting flags 0x4
D (1263) intr_alloc: Connected src 14 to int 19 (cpu 0)
V (1273) intr_alloc: esp_intr_alloc_intrstatus (cpu 0): checking args
V (1283) intr_alloc: esp_intr_alloc_intrstatus (cpu 0): Args okay. Resulting flags 0x4
D (1283) intr_alloc: Connected src 15 to int 20 (cpu 0)

More Information.

No response

@kristinpaget kristinpaget added the Type: Bug bugs in IDF label Aug 26, 2022
@espressif-bot espressif-bot added the Status: Opened Issue is new label Aug 26, 2022
@github-actions github-actions bot changed the title Silent failure of timer_isr_callback_add when using multiple timers Silent failure of timer_isr_callback_add when using multiple timers (IDFGH-8155) Aug 26, 2022
@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Aug 29, 2022
@suda-morris
Copy link
Collaborator

Thanks @kristinpaget for reporting this issue, we will propagate the failure caused by timer_isr_register.

@espressif-bot espressif-bot added Status: Reviewing Issue is being reviewed Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress Status: Reviewing Issue is being reviewed Resolution: NA Issue resolution is unavailable labels Aug 30, 2022
espressif-bot pushed a commit that referenced this issue Sep 3, 2022
Jason2866 added a commit to Jason2866/esp-idf that referenced this issue Sep 3, 2022
* timer: propagate isr register failure

Closes espressif#9651

* mcpwm: fix multiplication overflow in converting us to compare ticks

Closes espressif#9648

* heap: remove misleading info about malloc being equivalent to heap_caps_malloc(p, MALLOC_CAP_8BIT)

The actual memory allocated for malloc() depends on a lot of factors, see heap_caps_malloc_default()

Closes espressif#7659

* esp-rom: fixed error in miniz header documention for tdefl_init

Closes espressif#8435

* temperature_sensor: Fix issue that value is not accurate on ESP32-S3

* esp_https_ota: fix bug where `http_client_init_cb` is called after `esp_http_client_perform()` instead of before.

Closes espressif#9581

* Tasmota changes

* Fix linker error for C3

* Avoid bootloop if chip is unknown
   In case the PSIRAM chip is unknown, return an error and disable PSRAM
   instead of calling abort() and causing a bootloop
* Support for xiaomi single core ESP32
* Fix linker error for rom_temp_to_power
* fix linker error r_lld_ext_adv_dynamic_aux_pti_process

* Hide download percent when not interactive

* list(APPEND esptool_elf2image_args --dont-append-digest)

* Use native Apple ARM toolchains

* add package.json

* add submodules

* 8575d75

Co-authored-by: morris <maoshengrong@espressif.com>
Co-authored-by: Marius Vikhammer <marius.vikhammer@espressif.com>
Co-authored-by: Cao Sen Miao <caosenmiao@espressif.com>
Co-authored-by: Harshit Malpani <harshit.malpani@espressif.com>
Co-authored-by: Mahavir Jain <mahavir@espressif.com>
Jason2866 added a commit to Jason2866/esp-idf that referenced this issue Sep 3, 2022
* timer: propagate isr register failure

Closes espressif#9651

* mcpwm: fix multiplication overflow in converting us to compare ticks

Closes espressif#9648

* heap: remove misleading info about malloc being equivalent to heap_caps_malloc(p, MALLOC_CAP_8BIT)

The actual memory allocated for malloc() depends on a lot of factors, see heap_caps_malloc_default()

Closes espressif#7659

* esp-rom: fixed error in miniz header documention for tdefl_init

Closes espressif#8435

* temperature_sensor: Fix issue that value is not accurate on ESP32-S3

* esp_https_ota: fix bug where `http_client_init_cb` is called after `esp_http_client_perform()` instead of before.

Closes espressif#9581

* Tasmota changes

* Fix linker error for C3

* Avoid bootloop if chip is unknown
   In case the PSIRAM chip is unknown, return an error and disable PSRAM
   instead of calling abort() and causing a bootloop
* Support for xiaomi single core ESP32
* Fix linker error for rom_temp_to_power
* fix linker error r_lld_ext_adv_dynamic_aux_pti_process

* Hide download percent when not interactive

* list(APPEND esptool_elf2image_args --dont-append-digest)

* Use native Apple ARM toolchains

* add package.json

* add submodules

* 8575d75

* Arduino tinyusb v0.14.0 stripped

Co-authored-by: morris <maoshengrong@espressif.com>
Co-authored-by: Marius Vikhammer <marius.vikhammer@espressif.com>
Co-authored-by: Cao Sen Miao <caosenmiao@espressif.com>
Co-authored-by: Harshit Malpani <harshit.malpani@espressif.com>
Co-authored-by: Mahavir Jain <mahavir@espressif.com>
espressif-bot pushed a commit that referenced this issue Sep 10, 2022
@NRollo
Copy link

NRollo commented Sep 14, 2022

Maybe I have the same issue?
I also had the experience that only the first timer interrupt is trigged.
The timer is started in a task every ~10 sec. (vTaskDelay(pdMS_TO_TICKS(10000));) but only the first timer interrupt is serviced, the workaround was to do,
TIMERG0.hw_timer[0].config.alarm_en = 1;
when starting the timer (see below), the timer_enable_intr(TIMER_GROUP_0, TIMER_0); doesn't seem to have any effect at this point.
My understanding was that the esp-idf timer interrupt handler should take care of this?

Timer start in task:

  timer_set_counter_value(TIMER_GROUP_0, TIMER_0, (uint64_t) (PIDout/100.0 * 10000000));

  // This is apparently the only way to re-enable the timer interrupt???
  TIMERG0.hw_timer[0].config.alarm_en = 1;

  timer_start(TIMER_GROUP_0, TIMER_0);
  // Heating on
  gpio_set_level(HEATING, HEAT_ON);

Timer init:

    timer_config_t config = {
                                .divider = 80,
                                .counter_dir = TIMER_COUNT_DOWN,
                                .counter_en = TIMER_PAUSE,
                                .alarm_en = TIMER_ALARM_EN,
                                .auto_reload = false,
                            }; // default clock source is APB
    timer_init(TIMER_GROUP_0, TIMER_0, &config);
    
    // Configure the alarm value and the interrupt on alarm.
    timer_set_alarm_value(TIMER_GROUP_0, TIMER_0, 0);
    timer_enable_intr(TIMER_GROUP_0, TIMER_0);

    timer_isr_callback_add(TIMER_GROUP_0, TIMER_0, timer_group0_isr, (void *) TIMER_0, 0);

ISR:

bool IRAM_ATTR timer_group0_isr(void *para)
{
    // Stop the timer 
    timer_pause(TIMER_GROUP_0, TIMER_0);

    // Heating off
    gpio_set_level(HEATING, HEAT_OFF);

    return pdFALSE;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Bug bugs in IDF
Projects
None yet
Development

No branches or pull requests

4 participants