From 0e1dfa034d06652d9c425853d3b6c38d2cf6c828 Mon Sep 17 00:00:00 2001
From: Mike <mike@sillyhouse.net>
Date: Tue, 16 Jan 2024 08:31:07 +0000
Subject: [PATCH] Fix esp8266 timer1 testing (#2709)

This PR fixes an issue with `HostTests` which hangs consistently when testing Timer1 (used by HardwareTimer) on the Esp8266.

The problem occurs whilst checking each timer for consistency against system time.
This happens because the test code doesn't set a callback interrupt routine.
It is only a problem on the Esp8266 because the callback routine is set directly on the interrupt vector.
Other architectures use indirection and so do nothing if the callback isn't set.
NB. Doing this with the Esp8266 would increase interrupt latency which would affect PWM timing, etc.

Normal code uses the `HardwareTimer` class which contains logic to ensure this doesn't happen during setup.

Timer1 has only a 23-bit counter so fails test with /16 divisor as it wraps at around 1.7s (test duration is 2 seconds).
Fixed by restricting duration to timer maximum (less 1ms).

Elapsed tick calculation check also improved by first checking whether timer is an UP or DOWN counter,
and using timer maximum tick value to handle wraps correctly.

This PR also reduces the number of test loops on the Host (from 2000 to 50).

Note: Bug was introduced in #2456. Timer1 needed to be correctly configured (but inactive) to produce correct result for Esp32.
---
 Sming/Core/HardwareTimer.h         |  3 +--
 tests/HostTests/modules/Clocks.cpp | 38 ++++++++++++++++++++++++------
 2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/Sming/Core/HardwareTimer.h b/Sming/Core/HardwareTimer.h
index bf3677e0a2..92c9327fbc 100644
--- a/Sming/Core/HardwareTimer.h
+++ b/Sming/Core/HardwareTimer.h
@@ -141,11 +141,10 @@ class Timer1Api : public CallbackTimerApi<Timer1Api<clkdiv, mode>>
 template <hw_timer_clkdiv_t clkdiv, HardwareTimerMode mode> uint8_t Timer1Api<clkdiv, mode>::state;
 template <hw_timer_clkdiv_t clkdiv, HardwareTimerMode mode> uint32_t Timer1Api<clkdiv, mode>::interval;
 
-template <hw_timer_clkdiv_t clkdiv = TIMER_CLKDIV_16, HardwareTimerMode mode = eHWT_NonMaskable>
-
 /**
  * @brief Hardware Timer class template with selectable divider and interrupt mode
  */
+template <hw_timer_clkdiv_t clkdiv = TIMER_CLKDIV_16, HardwareTimerMode mode = eHWT_NonMaskable>
 using HardwareTimer1 = CallbackTimer<Timer1Api<clkdiv, mode>>;
 
 /**
diff --git a/tests/HostTests/modules/Clocks.cpp b/tests/HostTests/modules/Clocks.cpp
index 99163d6433..0c3cb62f08 100644
--- a/tests/HostTests/modules/Clocks.cpp
+++ b/tests/HostTests/modules/Clocks.cpp
@@ -31,7 +31,11 @@ template <class Clock, typename TimeType> class ClockTestTemplate : public TestG
 
 		printLimits();
 
-		for(unsigned i = 0; i < 2000; ++i) {
+		unsigned loopCount{2000};
+#ifdef ARCH_HOST
+		loopCount = 50;
+#endif
+		while(loopCount--) {
 			auto value = os_random();
 			check<NanoTime::Milliseconds>(value);
 			check<NanoTime::Microseconds>(value);
@@ -42,19 +46,39 @@ template <class Clock, typename TimeType> class ClockTestTemplate : public TestG
 
 		TEST_CASE("vs. system time")
 		{
-			constexpr uint32_t duration{2000000};
+			// Determine whether this is an up or down-counter
 			auto startTicks = Clock::ticks();
+			os_delay_us(100);
+			auto endTicks = Clock::ticks();
+			bool isDownCounter = (endTicks < startTicks);
+			debug_w("%s is %s counter", Clock::typeName(), isDownCounter ? "DOWN" : "UP");
+
+			// Run for a second or two and check timer ticks correspond approximately with system clock
+			constexpr uint64_t maxDuration = Clock::maxTicks().template as<NanoTime::Microseconds>() - 5000ULL;
+			constexpr uint32_t duration = std::min(2000000ULL, maxDuration);
 			auto startTime = system_get_time();
+			startTicks = Clock::ticks();
 			uint32_t time;
-			while((time = system_get_time()) < startTime + duration) {
+			while((time = system_get_time()) - startTime < duration) {
 				//
 			}
-			auto endTicks = Clock::ticks();
+			endTicks = Clock::ticks();
+			if(isDownCounter) {
+				std::swap(startTicks, endTicks);
+			}
+			uint32_t elapsedTicks = (endTicks - startTicks) % (Clock::maxTicks() + 1);
 
 			debug_w("System time elapsed: %u", time - startTime);
-			debug_w("%s ticks: %u", Clock::typeName(), endTicks - startTicks);
-			debug_w("Ratio: x %f", float(endTicks - startTicks) / (time - startTime));
-			debug_w("Apparent time: %u", uint32_t(Micros::ticksToTime(endTicks - startTicks)));
+			debug_w("Ticks: %u (%u - %u)", elapsedTicks, startTicks, endTicks);
+			debug_w("Ratio: x %f", float(elapsedTicks) / (time - startTime));
+			uint32_t us = Micros::ticksToTime(elapsedTicks);
+			debug_w("Apparent time: %u", us);
+#ifndef ARCH_HOST
+			// Up-timers may report 0 if inactive
+			if(endTicks != 0 || startTicks != 0) {
+				REQUIRE(abs(int(us - duration)) < 500); // Allow some latitude
+			}
+#endif
 		}
 	}