Skip to content

Commit

Permalink
Fix Esp32 hardware timer callbacks (#2716)
Browse files Browse the repository at this point in the history
Fix esp32 hardware timer callbacks

PR #2697 was supposed to fix the WDT from hardware timer interrupts.
It did this by disabling interrupts (and therefore callbacks) completely.

The actual reason for the WDT is twofold:

1. IDF version 5 changed some low-level HAL calls to accept bit masks instead of timer index.
2. Argument to interrupt callback wasn't stored or passed correctly so user-provided callback got junk for this parameter.

This PR fixes those issues and fixes incomplete testing in HostTests.

Also checked Basic_Blink sample using HardwareTimer.
  • Loading branch information
mikee47 authored Jan 31, 2024
1 parent fa90b7a commit 02a403f
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 9 deletions.
12 changes: 8 additions & 4 deletions Sming/Arch/Esp32/Components/driver/hw_timer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class TimerConfig
}

this->callback = callback;
this->arg = arg;

uint32_t status_reg = reinterpret_cast<uint32_t>(timer_ll_get_intr_status_reg(dev));
uint32_t mask = 1 << index;
Expand All @@ -62,9 +63,8 @@ class TimerConfig
#else
int source = timer_group_periph_signals.groups[group].timer_irq_id[index];
#endif
esp_intr_alloc_intrstatus(source, ESP_INTR_FLAG_IRAM | ESP_INTR_FLAG_INTRDISABLED, status_reg, mask, timerIsr,
this, &isr_handle);
clear_intr_status();
esp_intr_alloc_intrstatus(source, ESP_INTR_FLAG_IRAM, status_reg, mask, timerIsr, this, &isr_handle);
enable_intr(true);
}

Expand All @@ -84,13 +84,17 @@ class TimerConfig
timer_ll_intr_disable(dev, index);
}
#else
timer_ll_enable_intr(dev, index, state);
timer_ll_enable_intr(dev, TIMER_LL_EVENT_ALARM(index), state);
#endif
}

void __forceinline clear_intr_status()
{
#if ESP_IDF_VERSION_MAJOR < 5
timer_ll_clear_intr_status(dev, index);
#else
timer_ll_clear_intr_status(dev, TIMER_LL_EVENT_ALARM(index));
#endif
}

void __forceinline set_alarm_value(uint64_t value)
Expand Down Expand Up @@ -165,7 +169,7 @@ class TimerConfig
auto& timer = *static_cast<TimerConfig*>(arg);

if(timer.callback != nullptr) {
timer.callback(arg);
timer.callback(timer.arg);
}

timer.clear_intr_status();
Expand Down
4 changes: 4 additions & 0 deletions Sming/Arch/Esp32/Components/esp32/src/include/esp_clk.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
#pragma once

#include <c_types.h>
#if ESP_IDF_VERSION_MAJOR < 5
#include <hal/cpu_hal.h>
#else
#include <esp_cpu.h>
#endif
#include <sming_attr.h>
#include <esp_idf_version.h>

Expand Down
18 changes: 13 additions & 5 deletions tests/HostTests/modules/Timers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ class CallbackTimerTest : public TestGroup
{
System.queueCallback(
[](void* param) {
Serial.println(_F("timer1 expired"));
auto tmr = static_cast<HardwareTimerTest*>(param);
if(++tmr->count == 5) {
tmr->stop();
auto self = static_cast<CallbackTimerTest*>(param);
++self->timer1.count;
Serial << self->timer1.count << _F(" timer1 expired") << endl;
if(self->timer1.count == 5) {
self->timer1.stop();
self->timer1complete();
}
},
arg);
Expand All @@ -58,9 +60,15 @@ class CallbackTimerTest : public TestGroup
SHOW_SIZE(Timer1TestApi);
SHOW_SIZE(HardwareTimerTest);

timer1.initializeMs<500>(timer1Callback, &timer1);
timer1.initializeMs<500>(timer1Callback, this);
timer1.start();

Serial << _F("Waiting for timer1 callback test to complete") << endl;
pending();
}

void timer1complete()
{
statusTimer.initializeMs<1000>([this]() {
bool done = false;
++statusTimerCount;
Expand Down

0 comments on commit 02a403f

Please sign in to comment.