From 02a403f81344b5884f8cd5e385b1abb04e9325d8 Mon Sep 17 00:00:00 2001
From: Mike <mike@sillyhouse.net>
Date: Wed, 31 Jan 2024 13:22:13 +0000
Subject: [PATCH] Fix Esp32 hardware timer callbacks (#2716)

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.
---
 .../Arch/Esp32/Components/driver/hw_timer.cpp  | 12 ++++++++----
 .../Components/esp32/src/include/esp_clk.h     |  4 ++++
 tests/HostTests/modules/Timers.cpp             | 18 +++++++++++++-----
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/Sming/Arch/Esp32/Components/driver/hw_timer.cpp b/Sming/Arch/Esp32/Components/driver/hw_timer.cpp
index 84254b7cc0..ebba941797 100644
--- a/Sming/Arch/Esp32/Components/driver/hw_timer.cpp
+++ b/Sming/Arch/Esp32/Components/driver/hw_timer.cpp
@@ -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;
@@ -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);
 	}
 
@@ -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)
@@ -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();
diff --git a/Sming/Arch/Esp32/Components/esp32/src/include/esp_clk.h b/Sming/Arch/Esp32/Components/esp32/src/include/esp_clk.h
index 8579d2d67e..acf627a4e7 100644
--- a/Sming/Arch/Esp32/Components/esp32/src/include/esp_clk.h
+++ b/Sming/Arch/Esp32/Components/esp32/src/include/esp_clk.h
@@ -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>
 
diff --git a/tests/HostTests/modules/Timers.cpp b/tests/HostTests/modules/Timers.cpp
index 2dc7bb638c..361627e98d 100644
--- a/tests/HostTests/modules/Timers.cpp
+++ b/tests/HostTests/modules/Timers.cpp
@@ -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);
@@ -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;