Introduce Timebase module, refactor timing code + unit tests

CMakeLists.txt

@@ -203,6 +203,7 @@ target_sources(
           src/modules/motion.cpp
           src/modules/permanent_storage.cpp
           src/modules/selector.cpp
+          src/modules/timebase.cpp
           src/logic/command_base.cpp
           src/logic/cut_filament.cpp
           src/logic/eject_filament.cpp

src/main.cpp

@@ -267,15 +267,14 @@ void loop() {
     if (CheckMsgs()) {
         ProcessRequestMsg(protocol.GetRequestMsg());
     }
-    mb::buttons.Step(hal::adc::ReadADC(0));
-    ml::leds.Step(10);
-    mf::finda.Step(0);
-    mfs::fsensor.Step(0);
+    mb::buttons.Step();
+    ml::leds.Step();
+    mf::finda.Step();
+    mfs::fsensor.Step();
     mi::idler.Step();
     ms::selector.Step();
     currentCommand->Step();
     // add a watchdog reset
-    _delay_ms(10);
 }
 
 int main() {

src/modules/buttons.cpp

@@ -1,5 +1,6 @@
 #include "buttons.h"
 #include "../hal/adc.h"
+#include "timebase.h"
 
 namespace modules {
 namespace buttons {
@@ -22,7 +23,8 @@ int8_t Buttons::DecodeADC(uint16_t rawADC) {
     return -1;
 }
 
-void Buttons::Step(uint16_t millis) {
+void Buttons::Step() {
+    uint16_t millis = modules::time::timebase.Millis();
     int8_t currentState = DecodeADC(hal::adc::ReadADC(0));
     for (uint_fast8_t b = 0; b < N; ++b) {
         // this button was pressed if b == currentState, released otherwise

src/modules/buttons.h

@@ -32,7 +32,7 @@ public:
     inline constexpr Buttons() = default;
 
     /// State machine step - reads the ADC, processes debouncing, updates states of individual buttons
-    void Step(uint16_t millis);
+    void Step();
 
     /// @returns true if button at index is pressed
     /// @@TODO add range checking if necessary

src/modules/finda.cpp

@@ -1,13 +1,14 @@
 #include "finda.h"
 #include "../hal/adc.h"
+#include "timebase.h"
 
 namespace modules {
 namespace finda {
 
 FINDA finda;
 
-void FINDA::Step(uint16_t time) {
-    debounce::Debouncer::Step(time, hal::adc::ReadADC(1) > adcDecisionLevel);
+void FINDA::Step() {
+    debounce::Debouncer::Step(modules::time::timebase.Millis(), hal::adc::ReadADC(1) > adcDecisionLevel);
 }
 
 } // namespace finda

src/modules/finda.h

@@ -9,7 +9,7 @@ class FINDA : protected debounce::Debouncer {
 public:
     inline constexpr FINDA()
         : debounce::Debouncer(debounce) {};
-    void Step(uint16_t time);
+    void Step();
     using debounce::Debouncer::Pressed;
 
 private:

src/modules/fsensor.cpp

@@ -1,17 +1,18 @@
 #include "fsensor.h"
+#include "timebase.h"
 
 namespace modules {
 namespace fsensor {
 
 FSensor fsensor;
 
-void FSensor::Step(uint16_t time) {
-    debounce::Debouncer::Step(time, reportedFSensorState);
+void FSensor::Step() {
+    debounce::Debouncer::Step(modules::time::timebase.Millis(), reportedFSensorState);
 }
 
 void FSensor::ProcessMessage(bool on) {
     reportedFSensorState = on;
 }
 
-} // namespace finda
+} // namespace fsensor
 } // namespace modules

src/modules/fsensor.h

@@ -14,7 +14,7 @@ public:
     inline constexpr FSensor()
         : debounce::Debouncer(debounce)
         , reportedFSensorState(false) {};
-    void Step(uint16_t time);
+    void Step();
     using debounce::Debouncer::Pressed;
 
     void ProcessMessage(bool on);

src/modules/leds.cpp

@@ -1,5 +1,6 @@
 #include "leds.h"
 #include "../hal/shr16.h"
+#include "timebase.h"
 
 namespace modules {
 namespace leds {
@@ -39,9 +40,9 @@ bool LED::Step(bool oddPeriod) {
     return state.on;
 }
 
-void LEDs::Step(uint16_t delta_ms) {
-    ms += delta_ms;
-    bool oddPeriod = ((ms / 1000U) & 0x01U) != 0;
+void LEDs::Step() {
+    uint16_t millis = modules::time::timebase.Millis();
+    bool oddPeriod = ((millis / 1000U) & 0x01U) != 0;
     uint16_t result = 0;
     for (int8_t i = ledPairs * 2 - 1; i >= 0; --i) {
         result <<= 1;

src/modules/leds.h

@@ -53,11 +53,10 @@ private:
 /// main LED API
 class LEDs {
 public:
-    constexpr inline LEDs()
-        : ms(0) {};
+    constexpr inline LEDs() = default;
 
     /// step LED automaton
-    void Step(uint16_t delta_ms);
+    void Step();
 
     inline constexpr uint8_t LedPairsCount() const { return ledPairs; }
 
@@ -96,7 +95,6 @@ private:
     /// [8] - green LED slot 4
     /// [9] - red LED slot 4
     LED leds[ledPairs * 2];
-    uint16_t ms;
 };
 
 extern LEDs leds;

src/modules/timebase.cpp

@@ -0,0 +1,18 @@
+#include "timebase.h"
+#include "../hal/timers.h"
+
+namespace modules {
+namespace time {
+
+void Timebase::Init() {
+}
+
+void Timebase::ISR() {
+}
+
+uint16_t Timebase::Millis() const {
+    return ms;
+}
+
+} // namespace time
+} // namespace modules

src/modules/timebase.h

@@ -0,0 +1,28 @@
+#pragma once
+#include <stdint.h>
+
+namespace modules {
+namespace time {
+
+/// A basic time tracking class
+/// Works on top of processor timers and provides real-time steady clock
+/// (at least what the CPU thinks ;) )
+class Timebase {
+public:
+    constexpr inline Timebase()
+        : ms(0) {}
+    void Init();
+
+    /// @returns current milliseconds elapsed from the initialization of this class
+    ///  (usually the start of the firmware)
+    uint16_t Millis() const;
+
+private:
+    uint16_t ms;
+    static void ISR();
+};
+
+extern Timebase timebase;
+
+} // namespace time
+} // namespace modules

tests/unit/logic/cut_filament/CMakeLists.txt

@@ -17,6 +17,7 @@ add_executable(
   ../../modules/stubs/stub_adc.cpp
   ../../modules/stubs/stub_eeprom.cpp
   ../../modules/stubs/stub_shr16.cpp
+  ../../modules/stubs/stub_timebase.cpp
   ../stubs/main_loop_stub.cpp
   ../stubs/stub_motion.cpp
   test_cut_filament.cpp

tests/unit/logic/cut_filament/test_cut_filament.cpp

@@ -29,16 +29,16 @@ bool WhileCondition(COND cond, uint32_t maxLoops = 5000) {
 
 TEST_CASE("cut_filament::cut0", "[cut_filament]") {
     using namespace logic;
+
+    ForceReinitAllAutomata();
+
     CutFilament cf;
-    main_loop();
-
-    // let's assume we have the filament NOT loaded
-    modules::globals::globals.SetFilamentLoaded(false);
-
     // restart the automaton
     currentCommand = &cf;
     cf.Reset(0);
 
+    main_loop();
+
     // it should have instructed the selector and idler to move to slot 1
     // check if the idler and selector have the right command
     CHECK(modules::motion::axes[modules::motion::Idler].targetPos == 0); // @@TODO constants
@@ -49,7 +49,7 @@ TEST_CASE("cut_filament::cut0", "[cut_filament]") {
 
     // idler and selector reached their target positions and the CF automaton will start feeding to FINDA as the next step
     REQUIRE(cf.State() == ProgressCode::FeedingToFinda);
-    REQUIRE(WhileCondition([&]() { return cf.State() == ProgressCode::FeedingToFinda; }, 5000));
+    REQUIRE(WhileCondition([&]() { return cf.State() == ProgressCode::FeedingToFinda; }, 50000));
 
     // filament fed into FINDA, cutting...
     REQUIRE(cf.State() == ProgressCode::PreparingBlade);

tests/unit/logic/feed_to_finda/CMakeLists.txt

@@ -14,6 +14,7 @@ add_executable(
   ../../modules/stubs/stub_adc.cpp
   ../../modules/stubs/stub_eeprom.cpp
   ../../modules/stubs/stub_shr16.cpp
+  ../../modules/stubs/stub_timebase.cpp
   ../stubs/main_loop_stub.cpp
   ../stubs/stub_motion.cpp
   test_feed_to_finda.cpp

tests/unit/logic/stubs/main_loop_stub.cpp

@@ -1,6 +1,7 @@
 #include "main_loop_stub.h"
 
 #include "../../modules/stubs/stub_adc.h"
+#include "../../modules/stubs/stub_timebase.h"
 
 #include "../../../../src/modules/buttons.h"
 #include "../../../../src/modules/finda.h"
@@ -16,20 +17,18 @@
 
 logic::CommandBase *currentCommand = nullptr;
 
-// just like in the real FW, step all the known automata
-uint16_t millis = 0;
-
 void main_loop() {
-    modules::buttons::buttons.Step(millis);
-    modules::leds::leds.Step(millis);
-    modules::finda::finda.Step(millis);
-    modules::fsensor::fsensor.Step(millis);
+    modules::buttons::buttons.Step();
+    modules::leds::leds.Step();
+    modules::finda::finda.Step();
+    modules::fsensor::fsensor.Step();
     modules::idler::idler.Step();
     modules::selector::selector.Step();
     modules::motion::motion.Step();
     if (currentCommand)
         currentCommand->Step();
-    ++millis;
+
+    modules::time::IncMillis();
 }
 
 void ForceReinitAllAutomata() {
@@ -58,6 +57,8 @@ void ForceReinitAllAutomata() {
     // finda OFF
     hal::adc::ReinitADC(1, hal::adc::TADCData({ 0 }), 1);
 
+    modules::time::ReinitTimebase();
+
     // let's assume we have the filament NOT loaded and active slot 0
     modules::globals::globals.SetFilamentLoaded(false);
     modules::globals::globals.SetActiveSlot(0);

tests/unit/modules/buttons/CMakeLists.txt

@@ -1,7 +1,7 @@
 # define the test executable
 add_executable(
   buttons_tests ../../../../src/modules/buttons.cpp ../../../../src/modules/debouncer.cpp
-                ../stubs/stub_adc.cpp test_buttons.cpp
+                ../stubs/stub_adc.cpp ../stubs/stub_timebase.cpp test_buttons.cpp
   )
 
 # define required search paths

tests/unit/modules/buttons/test_buttons.cpp

@@ -1,26 +1,33 @@
 #include "catch2/catch.hpp"
 #include "buttons.h"
 #include "../stubs/stub_adc.h"
+#include "../stubs/stub_timebase.h"
 
 using Catch::Matchers::Equals;
 
 uint16_t millis = 0;
 
 bool Step_Basic_One_Button_Test(modules::buttons::Buttons &b, uint8_t oversampleFactor, uint8_t testedButtonIndex, uint8_t otherButton1, uint8_t otherButton2) {
-    for (uint8_t i = 0; i < oversampleFactor; ++i)
-        b.Step(++millis); // should detect the press but remain in detected state - wait for debounce
+    for (uint8_t i = 0; i < oversampleFactor; ++i) {
+        b.Step(); // should detect the press but remain in detected state - wait for debounce
+        modules::time::IncMillis();
+    }
     CHECK(!b.ButtonPressed(testedButtonIndex));
     CHECK(!b.ButtonPressed(otherButton1));
     CHECK(!b.ButtonPressed(otherButton2));
 
-    for (uint8_t i = 0; i < oversampleFactor; ++i)
-        b.Step(++millis); // reset to waiting
+    for (uint8_t i = 0; i < oversampleFactor; ++i) {
+        b.Step(); // reset to waiting
+        modules::time::IncMillis();
+    }
     CHECK(b.ButtonPressed(testedButtonIndex));
     CHECK(!b.ButtonPressed(otherButton1));
     CHECK(!b.ButtonPressed(otherButton2));
 
-    for (uint8_t i = 0; i < oversampleFactor; ++i)
-        b.Step(++millis); // pressed again, still in debouncing state
+    for (uint8_t i = 0; i < oversampleFactor; ++i) {
+        b.Step(); // pressed again, still in debouncing state
+        modules::time::IncMillis();
+    }
     CHECK(!b.ButtonPressed(testedButtonIndex));
     CHECK(!b.ButtonPressed(otherButton1));
     CHECK(!b.ButtonPressed(otherButton2));
@@ -31,7 +38,7 @@ bool Step_Basic_One_Button_Test(modules::buttons::Buttons &b, uint8_t oversample
 /// This test verifies the behaviour of a single button. The other buttons must remain intact.
 bool Step_Basic_One_Button(hal::adc::TADCData &&d, uint8_t testedButtonIndex) {
     using namespace modules::buttons;
-    millis = 0;
+    modules::time::ReinitTimebase();
     Buttons b;
 
     // need to oversample the data as debouncing takes 100 cycles to accept a pressed button
@@ -95,68 +102,87 @@ TEST_CASE("buttons::Step-debounce-one-button", "[buttons]") {
     // need to oversample the data as debouncing takes 100 cycles to accept a pressed button
     constexpr uint8_t oversampleFactor = 25;
     hal::adc::ReinitADC(0, std::move(d), oversampleFactor);
+    modules::time::ReinitTimebase();
 
     Buttons b;
 
     // 5
-    for (uint8_t i = 0; i < oversampleFactor; ++i)
-        b.Step(++millis); // should detect the press but remain in detected state - wait for debounce
+    for (uint8_t i = 0; i < oversampleFactor; ++i) {
+        b.Step(); // should detect the press but remain in detected state - wait for debounce
+        modules::time::IncMillis();
+    }
     CHECK(!b.ButtonPressed(0));
     CHECK(!b.ButtonPressed(1));
     CHECK(!b.ButtonPressed(2));
 
     // 1023
-    for (uint8_t i = 0; i < oversampleFactor; ++i)
-        b.Step(++millis); // reset to waiting
+    for (uint8_t i = 0; i < oversampleFactor; ++i) {
+        b.Step(); // reset to waiting
+        modules::time::IncMillis();
+    }
     CHECK(!b.ButtonPressed(0));
     CHECK(!b.ButtonPressed(1));
     CHECK(!b.ButtonPressed(2));
 
     // 5
-    for (uint8_t i = 0; i < oversampleFactor; ++i)
-        b.Step(++millis); // pressed again, still in debouncing state
+    for (uint8_t i = 0; i < oversampleFactor; ++i) {
+        b.Step(); // pressed again, still in debouncing state
+        modules::time::IncMillis();
+    }
     CHECK(!b.ButtonPressed(0));
     CHECK(!b.ButtonPressed(1));
     CHECK(!b.ButtonPressed(2));
 
     // 9
-    for (uint8_t i = 0; i < oversampleFactor; ++i)
-        b.Step(++millis); // no change
+    for (uint8_t i = 0; i < oversampleFactor; ++i) {
+        b.Step(); // no change
+        modules::time::IncMillis();
+    }
     CHECK(!b.ButtonPressed(0));
     CHECK(!b.ButtonPressed(1));
     CHECK(!b.ButtonPressed(2));
 
     // 6
-    for (uint8_t i = 0; i < oversampleFactor; ++i)
-        b.Step(++millis); // no change
+    for (uint8_t i = 0; i < oversampleFactor; ++i) {
+        b.Step(); // no change
+        modules::time::IncMillis();
+    }
     CHECK(!b.ButtonPressed(0));
     CHECK(!b.ButtonPressed(1));
     CHECK(!b.ButtonPressed(2));
 
     // 7
-    for (uint8_t i = 0; i < oversampleFactor; ++i)
-        b.Step(++millis); // one step from "pressed"
+    for (uint8_t i = 0; i < oversampleFactor; ++i) {
+        b.Step(); // one step from "pressed"
+        modules::time::IncMillis();
+    }
     CHECK(!b.ButtonPressed(0));
     CHECK(!b.ButtonPressed(1));
     CHECK(!b.ButtonPressed(2));
 
     // 8
-    for (uint8_t i = 0; i < oversampleFactor; ++i)
-        b.Step(++millis); // fifth set of samples - should report "pressed" finally
+    for (uint8_t i = 0; i < oversampleFactor; ++i) {
+        b.Step(); // fifth set of samples - should report "pressed" finally
+        modules::time::IncMillis();
+    }
     CHECK(b.ButtonPressed(0));
     CHECK(!b.ButtonPressed(1));
     CHECK(!b.ButtonPressed(2));
 
     // 1023
-    for (uint8_t i = 0; i < oversampleFactor; ++i)
-        b.Step(++millis); // sixth set of samples - button released (no debouncing on release)
+    for (uint8_t i = 0; i < oversampleFactor; ++i) {
+        b.Step(); // sixth set of samples - button released (no debouncing on release)
+        modules::time::IncMillis();
+    }
     CHECK(!b.ButtonPressed(0));
     CHECK(!b.ButtonPressed(1));
     CHECK(!b.ButtonPressed(2));
 
     // 1023
-    for (uint8_t i = 0; i < oversampleFactor; ++i)
-        b.Step(++millis); // seventh set of samples - still released
+    for (uint8_t i = 0; i < oversampleFactor; ++i) {
+        b.Step(); // seventh set of samples - still released
+        modules::time::IncMillis();
+    }
     CHECK(!b.ButtonPressed(0));
     CHECK(!b.ButtonPressed(1));
     CHECK(!b.ButtonPressed(2));

tests/unit/modules/leds/CMakeLists.txt

@@ -1,5 +1,8 @@
 # define the test executable
-add_executable(leds_tests ../../../../src/modules/leds.cpp ../stubs/stub_shr16.cpp test_leds.cpp)
+add_executable(
+  leds_tests ../../../../src/modules/leds.cpp ../stubs/stub_shr16.cpp ../stubs/stub_timebase.cpp
+             test_leds.cpp
+  )
 
 # define required search paths
 target_include_directories(

tests/unit/modules/leds/test_leds.cpp

@@ -1,9 +1,12 @@
 #include "catch2/catch.hpp"
 #include "leds.h"
 #include "shr16.h"
+#include "../stubs/stub_timebase.h"
 
 using Catch::Matchers::Equals;
 
+uint16_t millis = 0;
+
 namespace hal {
 namespace shr16 {
 extern uint16_t shr16_v_copy;
@@ -25,6 +28,7 @@ extern uint16_t shr16_v_copy;
 TEST_CASE("leds::single", "[leds]") {
     using namespace modules::leds;
     using namespace hal::shr16;
+    modules::time::ReinitTimebase(); // reset timing
 
     LEDs leds;
 
@@ -47,13 +51,15 @@ TEST_CASE("leds::single", "[leds]") {
 
     // turn LED on
     leds.SetMode(index, color, on);
-    leds.Step(0);
+    leds.Step();
+    modules::time::IncMillis();
     CHECK(leds.LedOn(index, color) == true);
     CHECK(shr16_v_copy == shr16_register);
 
     // turn LED off
     leds.SetMode(index, color, off);
-    leds.Step(0);
+    leds.Step();
+    modules::time::IncMillis();
     CHECK(leds.LedOn(index, color) == false);
     CHECK(shr16_v_copy == 0);
 }
@@ -61,10 +67,12 @@ TEST_CASE("leds::single", "[leds]") {
 void TestBlink(uint8_t index, modules::leds::Color color, uint16_t shr16_register, bool shouldBeOn, modules::leds::Mode blinkMode) {
     using namespace modules::leds;
     using namespace hal::shr16;
+    modules::time::ReinitTimebase(); // reset timing
     LEDs leds;
 
     leds.SetMode(index, color, blinkMode);
-    leds.Step(1);
+    leds.Step();
+    modules::time::IncMillis();
 
     REQUIRE(leds.LedOn(index, color) == shouldBeOn);
     CHECK(shr16_v_copy == (shouldBeOn ? shr16_register : 0));
@@ -72,7 +80,8 @@ void TestBlink(uint8_t index, modules::leds::Color color, uint16_t shr16_registe
     // test 4 seconds of blinking
     for (uint8_t s = 1; s < 4; ++s) {
         // one second elapsed ;)
-        leds.Step(1000);
+        modules::time::IncMillis(1000);
+        leds.Step();
         shouldBeOn = !shouldBeOn;
         CHECK(leds.LedOn(index, color) == shouldBeOn);
         CHECK(shr16_v_copy == (shouldBeOn ? shr16_register : 0));
@@ -80,7 +89,8 @@ void TestBlink(uint8_t index, modules::leds::Color color, uint16_t shr16_registe
 
     // turn LED off
     leds.SetMode(index, color, off);
-    leds.Step(0);
+    leds.Step();
+    modules::time::IncMillis();
     CHECK(leds.LedOn(index, color) == false);
     CHECK(shr16_v_copy == 0);
 }
@@ -88,7 +98,6 @@ void TestBlink(uint8_t index, modules::leds::Color color, uint16_t shr16_registe
 TEST_CASE("leds::blink0-single", "[leds]") {
     using namespace modules::leds;
     using namespace hal::shr16;
-
     uint8_t index;
     Color color;
     uint16_t shr16_register;

tests/unit/modules/stubs/stub_timebase.cpp

@@ -0,0 +1,28 @@
+#include "stub_timebase.h"
+#include "../../../../src/modules/timebase.h"
+
+namespace modules {
+namespace time {
+
+Timebase timebase;
+
+uint16_t millis = 0;
+
+void Timebase::Init() {}
+
+void Timebase::ISR() {}
+
+uint16_t Timebase::Millis() const {
+    return millis;
+}
+
+void ReinitTimebase(uint16_t ms /* = 0 */) {
+    millis = ms;
+}
+
+void IncMillis(uint16_t inc /* = 1*/) {
+    millis += inc;
+}
+
+} // namespace time
+} // namespace modules

tests/unit/modules/stubs/stub_timebase.h

@@ -0,0 +1,13 @@
+#pragma once
+
+#include <stdint.h>
+#include <vector>
+
+namespace modules {
+namespace time {
+
+extern void ReinitTimebase(uint16_t ms = 0);
+extern void IncMillis(uint16_t inc = 1);
+
+} // namespace time
+} // namespace modules