Merge remote-tracking branch 'upstream/main' into motion_wip

.pre-commit-config.yaml

@@ -5,7 +5,7 @@ repos:
   rev: 'v0.6.0'
   hooks:
     - id: cmake-format  # cmake formatter
-      files: ^(CMakeLists.*|.*\.cmake|.*\.cmake.in)$
+      files: (^|/)(CMakeLists.*|.*\.cmake|.*\.cmake.in)$
 - repo: https://github.com/pre-commit/mirrors-yapf
   rev: 'v0.27.0'
   hooks:

src/config/config.h

@@ -18,14 +18,14 @@ static constexpr const uint16_t fsensorDebounceMs = 10;
 
 // FINDA setup
 static constexpr const uint16_t findaDebounceMs = 100;
-static constexpr const uint8_t findaADCIndex = 1; ///< ADC index of FINDA input
+static constexpr const uint8_t findaADCIndex = 6; ///< ADC index of FINDA input
 static constexpr const uint16_t findaADCDecisionLevel = 512; ///< ADC decision level when a FINDA is considered pressed/not pressed
 
 // Buttons setup
 static constexpr const uint8_t buttonCount = 3; ///< number of buttons currently supported
 static constexpr const uint16_t buttonsDebounceMs = 100;
-static constexpr const uint16_t buttonADCLimits[buttonCount][2] = { { 0, 10 }, { 320, 360 }, { 500, 530 } };
+static constexpr const uint16_t buttonADCLimits[buttonCount][2] = { { 0, 50 }, { 80, 100 }, { 160, 180 } };
-static constexpr const uint8_t buttonsADCIndex = 0; ///< ADC index of buttons input
+static constexpr const uint8_t buttonsADCIndex = 5; ///< ADC index of buttons input
 
 /// Maximum microstepping resolution. This defines the effective unit of
 /// the step intevals on the motion API, independently of the selected

src/hal/adc.cpp

@@ -1,9 +1,31 @@
 #include "adc.h"
+#include <avr/io.h>
 
 namespace hal {
 namespace adc {
 
-uint16_t ReadADC(uint8_t adc) { return 0; }
+void Init() {
+    ADCSRA |= (1 << ADPS2) | (1 << ADPS1) | (1 << ADPS0);
+	ADMUX |= (1 << REFS0);
+    ADCSRA |= (1 << ADEN);
+}
+
+uint16_t ReadADC(uint8_t channel) { 
+    uint8_t admux = ADMUX;
+    admux &= ~0x1F;
+    admux |= channel & 0x07;
+    ADMUX = admux;
+
+    uint8_t adcsrb = ADCSRB;
+    adcsrb &= ~(1 << MUX5);
+    adcsrb |= (channel & 0x08) << 1;
+    ADCSRB = adcsrb;
+
+    ADCSRA |= (1 << ADSC);
+    while (ADCSRA & (1 << ADSC));
+
+    return ADC;
+ }
 
 } // namespace adc
 } // namespace hal

src/hal/adc.h

@@ -7,7 +7,8 @@ namespace hal {
 namespace adc {
 
 /// ADC access routines
-uint16_t ReadADC(uint8_t adc);
+void Init();
+uint16_t ReadADC(uint8_t channel);
 
 } // namespace adc
 } // namespace hal

src/main.cpp

@@ -137,7 +137,7 @@ void setup() {
     ml::leds.SetMode(1, ml::Color::green, ml::Mode::on);
     ml::leds.Step();
 
-    // adc::Init();
+    adc::Init();
     ml::leds.SetMode(0, ml::Color::green, ml::Mode::on);
     ml::leds.Step();
 }

tests/CMakeLists.txt

@@ -1 +1,43 @@
+option(GCOV_ENABLE "Enable/use GCOV code coverage for tests" ON)
+
+if (GCOV_ENABLE)
+	find_program(GCOV_BINARY NAMES gcov gcov-7)
+	find_program(LCOV_BINARY NAMES lcov)
+	message(STATUS "Using gcov ${GCOV_BINARY} and LCOV ${LCOV_BINARY}")
+	add_compile_options(-g -O0 -fprofile-arcs -ftest-coverage)
+	link_libraries(-coverage -lgcov)
+	set(CMAKE_CXX_OUTPUT_EXTENSION_REPLACE ON)
+	add_custom_target(tests_clean
+		WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
+		COMMAND ${CMAKE_COMMAND} -E remove_directory ${PROJECT_BINARY_DIR}/Coverage
+		COMMAND ${LCOV_BINARY} -b ${PROJECT_BINARY_DIR} -d ${PROJECT_BINARY_DIR} --zerocounters
+		COMMAND ${CMAKE_COMMAND} -E remove ${PROJECT_BINARY_DIR}/.ctest-finished
+	)
+
+	# This step needs to always return OK but log whether it was successful or not. The thought here is that if the tests all
+	# pass, .ctest-finished is created and we can check for its existance after generating the report to determine if the overall
+	# build result is a pass or fail. 
+	add_custom_target(test_run_all
+		COMMAND cd ${PROJECT_BINARY_DIR} 
+		COMMAND env CTEST_OUTPUT_ON_FAILURE=1 ctest --timeout 30 && ${CMAKE_COMMAND} -E touch .ctest-finished || exit 0
+		DEPENDS tests_clean tests
+	)
+
+	add_custom_target(test_coverage_report
+		COMMAND cd ${PROJECT_BINARY_DIR}
+		COMMAND ${LCOV_BINARY} --capture --gcov-tool="${GCOV_BINARY}" --directory ${PROJECT_BINARY_DIR} --output-file ${PROJECT_BINARY_DIR}/coverage.info
+		# Strip system headers and other uninteresting stuff. 
+		COMMAND ${LCOV_BINARY} --remove coverage.info '/usr/*' '${PROJECT_SOURCE_DIR}/tests/*' '${PROJECT_SOURCE_DIR}/lib/Catch2/*' -o coverage.info
+		# Package it up. 
+		COMMAND genhtml coverage.info --output-directory Coverage
+		COMMAND tar -zcvf Coverage.tar.gz Coverage
+		# Cheat and compare a file to itself to check for existence. File-Not-Found is a failure code. 
+		COMMAND ../../utils/gcovr.py -r . -e '../../tests' -e '../../lib/Catch2' | tee Summary.txt
+		COMMAND ${CMAKE_COMMAND} -E compare_files ${PROJECT_BINARY_DIR}/.ctest-finished ${PROJECT_BINARY_DIR}/.ctest-finished
+		DEPENDS test_run_all
+	)
+else()
+	set(CMAKE_CXX_OUTPUT_EXTENSION_REPLACE OFF)
+endif()
+
 add_subdirectory(unit)

tests/unit/modules/buttons/test_buttons.cpp

@@ -1,11 +1,9 @@
 #include "catch2/catch.hpp"
-#include "buttons.h"
 #include "../stubs/stub_adc.h"
 #include "../stubs/stub_timebase.h"
+#include "buttons.h"
 
-using Catch::Matchers::Equals;
+static constexpr const uint16_t adcMaxValue = 1023U;
-
-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) {
@@ -17,9 +15,20 @@ bool Step_Basic_One_Button_Test(modules::buttons::Buttons &b, uint8_t oversample
     CHECK(!b.ButtonPressed(otherButton2));
 
     for (uint8_t i = 0; i < oversampleFactor; ++i) {
-        b.Step(); // reset to waiting
+        b.Step();
         modules::time::IncMillis();
     }
+
+    // just before the debounce trigger
+    CHECK(!b.ButtonPressed(testedButtonIndex));
+    CHECK(!b.ButtonPressed(otherButton1));
+    CHECK(!b.ButtonPressed(otherButton2));
+
+    // Tune the alg to overcome an edge case in debouncing timing - just in the unit test
+    // This is very brittle, needs some work @TODO to clean up
+    modules::time::IncMillis(4);
+    b.Step(); // reset to waiting
+
     CHECK(b.ButtonPressed(testedButtonIndex));
     CHECK(!b.ButtonPressed(otherButton1));
     CHECK(!b.ButtonPressed(otherButton2));
@@ -37,12 +46,12 @@ 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;
+    namespace mb = modules::buttons;
     modules::time::ReinitTimebase();
-    Buttons b;
+    mb::Buttons b;
 
     // need to oversample the data as debouncing takes 100 cycles to accept a pressed button
-    constexpr uint8_t oversampleFactor = 100;
+    constexpr uint8_t oversampleFactor = config::buttonsDebounceMs;
     hal::adc::ReinitADC(config::buttonsADCIndex, std::move(d), oversampleFactor);
 
     uint8_t otherButton1 = 1, otherButton2 = 2;
@@ -61,17 +70,11 @@ bool Step_Basic_One_Button(hal::adc::TADCData &&d, uint8_t testedButtonIndex) {
 }
 
 TEST_CASE("buttons::Step-basic-button", "[buttons]") {
-    {
+    for (uint8_t i = 0; i < config::buttonCount; ++i) {
-        hal::adc::TADCData d({ 5, 6, 1023 });
+        CHECK(Step_Basic_One_Button({ config::buttonADCLimits[i][0],
-        CHECK(Step_Basic_One_Button(std::move(d), 0));
+                                        config::buttonADCLimits[i][1],
-    }
+                                        adcMaxValue },
-    {
+            i));
-        hal::adc::TADCData d({ 321, 359, 1023 });
-        CHECK(Step_Basic_One_Button(std::move(d), 1));
-    }
-    {
-        hal::adc::TADCData d({ 501, 529, 1023 });
-        CHECK(Step_Basic_One_Button(std::move(d), 2));
     }
 }
 
@@ -79,12 +82,14 @@ TEST_CASE("buttons::Step-basic-button", "[buttons]") {
 /// and the Buttons class should press first button and release, then the second one and then the third one
 /// without being reinitialized.
 TEST_CASE("buttons::Step-basic-button-one-after-other", "[buttons]") {
-    using namespace modules::buttons;
+    namespace mb = modules::buttons;
-    hal::adc::TADCData d({ 5, 6, 1023, 321, 359, 1023, 501, 529, 1023 });
+    hal::adc::TADCData d({ config::buttonADCLimits[0][0], config::buttonADCLimits[0][0] + 1, adcMaxValue,
-    Buttons b;
+        config::buttonADCLimits[1][0], config::buttonADCLimits[1][0] + 1, adcMaxValue,
+        config::buttonADCLimits[2][0], config::buttonADCLimits[2][0] + 1, adcMaxValue });
+    mb::Buttons b;
 
     // need to oversample the data as debouncing takes 100 cycles to accept a pressed button
-    constexpr uint8_t oversampleFactor = 100;
+    constexpr uint8_t oversampleFactor = config::buttonsDebounceMs;
     hal::adc::ReinitADC(config::buttonsADCIndex, std::move(d), oversampleFactor);
 
     CHECK(Step_Basic_One_Button_Test(b, oversampleFactor, 0, 1, 2));
@@ -94,17 +99,17 @@ TEST_CASE("buttons::Step-basic-button-one-after-other", "[buttons]") {
 
 /// This test tries to simulate a bouncing effect on data from ADC on the first button
 TEST_CASE("buttons::Step-debounce-one-button", "[buttons]") {
-    using namespace modules::buttons;
+    namespace mb = modules::buttons;
 
     // make a bounce event on the first press
-    hal::adc::TADCData d({ 5, 1023, 5, 9, 6, 7, 8, 1023, 1023 });
+    hal::adc::TADCData d({ 5, adcMaxValue, 5, 9, 6, 7, 8, adcMaxValue, adcMaxValue });
 
     // need to oversample the data as debouncing takes 100 cycles to accept a pressed button
     constexpr uint8_t oversampleFactor = 25;
     hal::adc::ReinitADC(config::buttonsADCIndex, std::move(d), oversampleFactor);
     modules::time::ReinitTimebase();
 
-    Buttons b;
+    mb::Buttons b;
 
     // 5
     for (uint8_t i = 0; i < oversampleFactor; ++i) {

tests/unit/modules/stubs/stub_adc.cpp

@@ -5,8 +5,8 @@
 namespace hal {
 namespace adc {
 
-static TADCData values2Return[2];
+static TADCData values2Return[6];
-static TADCData::const_iterator rdptr[2] = { values2Return[0].cbegin(), values2Return[1].cbegin() };
+static TADCData::const_iterator rdptr[6] = { values2Return[0].cbegin(), values2Return[1].cbegin() };
 static uint8_t oversampleFactor = 1;
 static uint8_t oversample = 1; ///< current count of oversampled values returned from the ADC - will get filled with oversampleFactor once it reaches zero