From 5b9c6ec36eb3026a1dfa2e562acd7dd879164805 Mon Sep 17 00:00:00 2001
From: "D.R.racer" <drracer@drracer.eu>
Date: Fri, 15 Oct 2021 07:10:19 +0200
Subject: [PATCH] Fix (workaround) LED blink when millis overflow + unit test

---
 src/config/config.h                   |  6 ++++-
 tests/unit/modules/leds/test_leds.cpp | 35 +++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/config/config.h b/src/config/config.h
index 8272dad..09cc780 100644
--- a/src/config/config.h
+++ b/src/config/config.h
@@ -24,7 +24,11 @@ static constexpr const uint16_t fsensorDebounceMs = 10;
 
 // LEDS
 /// The complete period of LED's blinking (i.e. ON and OFF together)
-static constexpr uint16_t ledBlinkPeriodMs = 1000U;
+/// Beware - in order to keep the blink periods "handle" millis overflow seamlessly
+/// keep the period a power of 2 (i.e. 256, 512, 1024).
+/// If you don't, one of the LED unit tests will fail.
+static constexpr uint16_t ledBlinkPeriodMs = 1024U;
+static_assert(ledBlinkPeriodMs == 256 || ledBlinkPeriodMs == 512 || ledBlinkPeriodMs == 1024 || ledBlinkPeriodMs == 2048, "LED blink period should be a power of 2");
 
 // FINDA setup
 static constexpr const uint16_t findaDebounceMs = 100;
diff --git a/tests/unit/modules/leds/test_leds.cpp b/tests/unit/modules/leds/test_leds.cpp
index 2e659b3..c8ad37b 100644
--- a/tests/unit/modules/leds/test_leds.cpp
+++ b/tests/unit/modules/leds/test_leds.cpp
@@ -2,6 +2,7 @@
 #include "leds.h"
 #include "shr16.h"
 #include "../stubs/stub_timebase.h"
+#include "../../../../src/modules/timebase.h"
 
 using Catch::Matchers::Equals;
 
@@ -117,3 +118,37 @@ TEST_CASE("leds::blink0-single", "[leds]") {
 
     TestBlink(index, color, shr16_register, true, ml::blink1);
 }
+
+void TestBlinkOverflow(uint16_t period) {
+    using namespace hal::shr16;
+
+    // reset timing to a stage close to millis overflow
+    mt::ReinitTimebase(0x10000U - period / 2 + 1);
+
+    ml::LEDs leds;
+    uint8_t index = 0;
+    ml::Color color = ml::green;
+    uint16_t ms = mt::timebase.Millis();
+    bool shouldBeOn = ((ms / (period / 2)) & 0x01U) != 0;
+    uint16_t shr16_register = SHR16_LEDG0;
+
+    leds.SetMode(index, color, ml::blink0);
+    leds.Step();
+
+    REQUIRE(leds.LedOn(index, color) == shouldBeOn);
+    CHECK(shr16_v_copy == (shouldBeOn ? shr16_register : 0));
+
+    // advance timebase with overflow
+    mt::IncMillis(period / 2);
+
+    // the LED must have changed its state
+    leds.Step();
+
+    REQUIRE(leds.LedOn(index, color) != shouldBeOn);
+    CHECK(shr16_v_copy == (shouldBeOn ? 0 : shr16_register));
+}
+
+TEST_CASE("leds::blink0-overflow", "[leds]") {
+    // this is a check for a correct FW configuration
+    TestBlinkOverflow(config::ledBlinkPeriodMs);
+}