Adding a per-target unit test for timer metadata consistency.

src/test/Makefile

@@ -13,15 +13,19 @@
 USER_DIR = ../main
 TEST_DIR = unit
 ROOT = ../..
+OBJECT_DIR = ../../obj/test
+TARGET_DIR = $(USER_DIR)/target
 
 include $(ROOT)/make/system-id.mk
+include $(ROOT)/make/targets_list.mk
 
 # specify which files that are included in the test in addition to the unittest file.
 # variables available:
 #   <test_name>_SRC
 #   <test_name>_DEFINES
 #   <test_name>_INCLUDE_DIRS
-
+#   <test_name>_EXPAND (run for each target, call the above with target as $1)
+#   <test_name>_BLACKLIST (targets to exclude from an expanded test's run)
 
 alignsensor_unittest_SRC := \
 		$(USER_DIR)/sensors/boardalignment.c \
@@ -39,9 +43,9 @@ atomic_unittest_SRC := \
 		$(USER_DIR)/build/atomic.c \
 		$(TEST_DIR)/atomic_unittest_c.c
 
-baro_bmp085_unittest_SRC := \
-		$(USER_DIR)/drivers/barometer/barometer_bmp085.c \
-		$(USER_DIR)/drivers/io.c
+#baro_bmp085_unittest_SRC := \
+#		$(USER_DIR)/drivers/barometer/barometer_bmp085.c \
+#		$(USER_DIR)/drivers/io.c
 
 
 baro_bmp280_unittest_SRC := \
@@ -58,9 +62,9 @@ baro_ms5611_unittest_DEFINES := \
                 USE_BARO_MS5611 \
                 USE_BARO_SPI_MS5611
 
-battery_unittest_SRC := \
-		$(USER_DIR)/sensors/battery.c \
-		$(USER_DIR)/common/maths.c
+#battery_unittest_SRC := \
+#		$(USER_DIR)/sensors/battery.c \
+#		$(USER_DIR)/common/maths.c
 
 
 blackbox_unittest_SRC :=  \
@@ -265,15 +269,24 @@ telemetry_ibus_unittest_SRC := \
 		$(USER_DIR)/telemetry/ibus_shared.c \
 		$(USER_DIR)/telemetry/ibus.c
 
+timer_definition_unittest_EXPAND := yes
+
+# SITL is a simulator; NERO and STM32F7X2 don't define USED_TIMERS in target.h.
+timer_definition_unittest_BLACKLIST := STM32F7X2 SITL NERO
+
+timer_definition_unittest_SRC = \
+		$(TARGET_DIR)/$(call get_base_target,$1)/target.c
+
+timer_definition_unittest_DEFINES = \
+		TARGET=$(call get_base_target,$1)
+
+timer_definition_unittest_INCLUDE_DIRS = \
+		$(TEST_DIR)/timer_definition_unittest.include \
+		$(TARGET_DIR)/$(call get_base_target,$1)
 
 transponder_ir_unittest_SRC := \
-	        $(USER_DIR)/drivers/transponder_ir_ilap.c \
-	        $(USER_DIR)/drivers/transponder_ir_arcitimer.c
-
-
-type_conversion_unittest_SRC := \
-		$(USER_DIR)/common/typeconversion.c
-
+		$(USER_DIR)/drivers/transponder_ir_ilap.c \
+		$(USER_DIR)/drivers/transponder_ir_arcitimer.c
 
 ws2811_unittest_SRC := \
 		$(USER_DIR)/drivers/light_ws2811strip.c
@@ -328,11 +341,6 @@ vtx_unittest_DEFINES := \
 # Remember to tweak this if you move this file.
 GTEST_DIR = ../../lib/test/gtest
 
-
-USER_INCLUDE_DIR = $(USER_DIR)
-
-OBJECT_DIR = ../../obj/test
-
 # Use clang/clang++ by default
 CC  := clang
 CXX := clang++
@@ -383,8 +391,10 @@ LDFLAGS  += -Wl,-T,$(TEST_DIR)/pg.ld -Wl,-Map,$(OBJECT_DIR)/$@.map
 endif
 
 # Gather up all of the tests.
-TEST_SRC = $(sort $(wildcard $(TEST_DIR)/*.cc))
-TESTS = $(TEST_SRC:$(TEST_DIR)/%.cc=%)
+TEST_SRCS = $(sort $(wildcard $(TEST_DIR)/*.cc))
+TEST_BASENAMES = $(TEST_SRCS:$(TEST_DIR)/%.cc=%)
+TESTS = $(foreach test,$(TEST_BASENAMES),$(if $($(test)_EXPAND),$(foreach \
+		target,$(filter-out $($(test)_BLACKLIST),$(VALID_TARGETS)),$(test).$(target)),$(test)))
 
 # All Google Test headers.  Usually you shouldn't change this
 # definition.
@@ -465,69 +475,119 @@ $(OBJECT_DIR)/gtest_main.a : $(OBJECT_DIR)/gtest-all.o $(OBJECT_DIR)/gtest_main.
          $(OBJECT_DIR)/gtest_main.d
 
 
-# includes in test dir must override includes in user dir
-TEST_INCLUDE_DIRS := $(TEST_DIR) \
-	$(USER_INCLUDE_DIR)
-
-TEST_CFLAGS	 = $(addprefix -I,$(TEST_INCLUDE_DIRS))
-
+# includes in test dir must override includes in user dir, unless the user
+# specifies a list of endorsed directories in ${target}_INCLUDE_DIRS.
+test_include_dirs  = $1 $(TEST_DIR) $(USER_DIR)
+test_cflags	   = $(addprefix -I,$(call test_include_dirs,$1))
 
 
+# target name extractor
+# param $1 = expanded test name in the form of test.target
+target = $(1:$(basename $1).%=%)
 
 # canned recipe for all test builds
-# param $1 = testname
+#
+# variable expansion rules of thumb (number of $'s):
+# * parameters: one $, e.g. $1
+# * statically accessed variables set elsewhere: one $, e.g. $(C_FLAGS)
+# * dynamically accessed variables set elsewhere: one $, e.g. $($1_SRC)
+# * make functions accessing only the above: one $, e.g. $(basename $1)
+# * dynamically set and accessed variables: two $, e.g. $$($1_OBJS)
+# * make functions accessing dynamically set variables: two $,
+#   e.g. $$(call test_cflags,$$($1_INCLUDE_DIRS))
+#
+# param $1 = plain test name for global tests, test.target for per-target tests
 define test-specific-stuff
 
-$$1_OBJS = $$(patsubst $$(TEST_DIR)%,$$(OBJECT_DIR)/$1%, $$(patsubst $$(USER_DIR)%,$$(OBJECT_DIR)/$1%,$$($1_SRC:=.o)))
+ifeq ($1,$(basename $1))
+# standard global test
+$1_OBJS = $(patsubst \
+	$(TEST_DIR)/%,$(OBJECT_DIR)/$1/%,$(patsubst \
+	$(USER_DIR)/%,$(OBJECT_DIR)/$1/%,$($1_SRC:=.o)))
+else
+# test executed for each target, $1 has the form of test.target
+$1_SRC = $(addsuffix .o,$(call $(basename $1)_SRC,$(call target,$1)))
+$1_OBJS = $$(patsubst \
+	$(TEST_DIR)/%,$(OBJECT_DIR)/$1/%,$$(patsubst \
+	$(USER_DIR)/%,$(OBJECT_DIR)/$1/%,$$(patsubst \
+	$(TARGET_DIR)/$(call get_base_target,$(call target,$1))/%,$(OBJECT_DIR)/$1/%,$$($1_SRC))))
+$1_DEFINES = $(call $(basename $1)_DEFINES,$(call target,$1))
+$1_INCLUDE_DIRS = $(call $(basename $1)_INCLUDE_DIRS,$(call target,$1))
+endif
 
 # $$(info $1 -v-v-------)
-# $$(info $1_SRC:  $($1_SRC))
-# $$(info $1_OBJS: $$($$1_OBJS))
+# $$(info $1_SRC:  $$($1_SRC))
+# $$(info $1_OBJS: $$($1_OBJS))
 # $$(info $1 -^-^-------)
 
-
-#include generated dependencies
--include $$($$1_OBJS:.o=.d)
--include $(OBJECT_DIR)/$1/$1.d
-
+# include generated dependencies
+-include $$($1_OBJS:.o=.d)
+-include $(OBJECT_DIR)/$1/$(basename $1).d
 
 $(OBJECT_DIR)/$1/%.c.o: $(USER_DIR)/%.c
 	@echo "compiling $$<" "$(STDOUT)"
 	$(V1) mkdir -p $$(dir $$@)
-	$(V1) $(CC) $(C_FLAGS) $(TEST_CFLAGS) \
-                $(foreach def,$($1_INCLUDE_DIRS),-I $(def)) \
-                $(foreach def,$($1_DEFINES),-D $(def)) \
+	$(V1) $(CC) $(C_FLAGS) $$(call test_cflags,$$($1_INCLUDE_DIRS)) \
+                $$(foreach def,$$($1_DEFINES),-D $$(def)) \
                 -c $$< -o $$@
 
 $(OBJECT_DIR)/$1/%.c.o: $(TEST_DIR)/%.c
 	@echo "compiling test c file: $$<" "$(STDOUT)"
 	$(V1) mkdir -p $$(dir $$@)
-	$(V1) $(CC) $(C_FLAGS) $(TEST_CFLAGS) \
-                $(foreach def,$($1_INCLUDE_DIRS),-I $(def)) \
-                $(foreach def,$($1_DEFINES),-D $(def)) \
+	$(V1) $(CC) $(C_FLAGS) $$(call test_cflags,$$($1_INCLUDE_DIRS)) \
+                $$(foreach def,$$($1_DEFINES),-D $$(def)) \
                 -c $$< -o $$@
 
-$(OBJECT_DIR)/$1/$1.o: $(TEST_DIR)/$1.cc
+ifneq ($1,$(basename $1))
+# per-target tests may compile files from the target directory
+$(OBJECT_DIR)/$1/%.c.o: $(TARGET_DIR)/$(call get_base_target,$(call target,$1))/%.c
+	@echo "compiling target c file: $$<" "$(STDOUT)"
+	$(V1) mkdir -p $$(dir $$@)
+	$(V1) $(CC) $(C_FLAGS) $$(call test_cflags,$$($1_INCLUDE_DIRS)) \
+                $$(foreach def,$$($1_DEFINES),-D $$(def)) \
+                -c $$< -o $$@
+endif
+
+$(OBJECT_DIR)/$1/$(basename $1).o: $(TEST_DIR)/$(basename $1).cc
 	@echo "compiling $$<" "$(STDOUT)"
 	$(V1) mkdir -p $$(dir $$@)
-	$(V1) $(CXX) $(CXX_FLAGS) $(TEST_CFLAGS)  \
-                $(foreach def,$($1_INCLUDE_DIRS),-I $(def)) \
-                $(foreach def,$($1_DEFINES),-D $(def)) \
+	$(V1) $(CXX) $(CXX_FLAGS) $$(call test_cflags,$$($1_INCLUDE_DIRS)) \
+                $$(foreach def,$$($1_DEFINES),-D $$(def)) \
                 -c $$< -o $$@
 
-
-$(OBJECT_DIR)/$1/$1 : $$($$1_OBJS) \
-    $(OBJECT_DIR)/$1/$1.o \
+$(OBJECT_DIR)/$1/$(basename $1): $$($1_OBJS) \
+	$(OBJECT_DIR)/$1/$(basename $1).o \
 	$(OBJECT_DIR)/gtest_main.a
 
 	@echo "linking $$@" "$(STDOUT)"
 	$(V1) mkdir -p $(dir $$@)
 	$(V1) $(CXX) $(CXX_FLAGS) $(LDFLAGS) $$^ -o $$@
 
-test_$1: $(OBJECT_DIR)/$1/$1
+test_$1: $(OBJECT_DIR)/$1/$(basename $1)
 	$(V1) $$< $$(EXEC_OPTS) "$(STDOUT)" && echo "running $$@: PASS"
 
 endef
 
-#apply the canned recipe above to all tests
+
 $(eval $(foreach test,$(TESTS),$(call test-specific-stuff,$(test))))
+
+$(foreach test,$(TESTS),$(if $($(basename $(test))_SRC),,$(error \
+	Test 'unit/$(basename $(test)).cc' has no '$(basename $(test))_SRC' variable defined)))
+$(foreach var,$(filter-out TARGET_SRC,$(filter %_SRC,$(.VARIABLES))),$(if $(filter $(var:_SRC=)%,$(TESTS)),,$(error \
+	Variable '$(var)' has no 'unit/$(var:_SRC=).cc' test)))
+
+
+target_list:
+	@echo ========== BASE TARGETS ==========
+	@echo $(BASE_TARGETS)
+	@echo ========== ALT TARGETS ==========
+	@echo $(ALT_TARGETS)
+	@echo ========== VALID_TARGETS ==========
+	@echo $(VALID_TARGETS)
+	@echo ========== BASE/ALT PAIRS ==========
+	@echo $(BASE_ALT_PAIRS)
+	@echo ========== ALT/BASE MAPPING ==========
+	@echo $(foreach target,$(ALT_TARGETS),$(target)\>$(call get_base_target,$(target)))
+	@echo ========== ALT/BASE FULL MAPPING ==========
+	@echo $(foreach target,$(VALID_TARGETS),$(target)\>$(call get_base_target,$(target)))
+

src/test/unit/timer_definition_unittest.cc

@@ -0,0 +1,103 @@
+/*
+ * This file is part of Betaflight.
+ *
+ * Betaflight is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Betaflight is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Betaflight.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+extern "C" {
+    #include <target.h>
+    #include <drivers/timer.h>
+    extern const timerHardware_t timerHardware[USABLE_TIMER_CHANNEL_COUNT];
+}
+
+#include <bitset>
+#include <iostream>
+#include <set>
+#include <sstream>
+#include <string>
+#include "gtest/gtest.h"
+
+TEST(TimerDefinitionTest, Test_counterMismatch) {
+    for (const timerHardware_t &t : timerHardware)
+        ASSERT_EQ(&t - timerHardware, t.def_tim_counter)
+            << "Counter mismatch in timerHardware (in target.c) at position "
+            << &t - timerHardware << "; the array may have uninitialized "
+            << "trailing elements. This happens when USABLE_TIMER_CHANNEL_COUNT"
+            << " is not equal to the number of array initializers. Current "
+            << "value is " << USABLE_TIMER_CHANNEL_COUNT << ", last initialized"
+            << " array element appears to be " << &t - timerHardware - 1 << '.';
+}
+
+TEST(TimerDefinitionTest, Test_duplicatePin) {
+    std::set<TestPinEnum> usedPins;
+    for (const timerHardware_t &t : timerHardware)
+        EXPECT_TRUE(usedPins.emplace(t.pin).second)
+            << "Pin " << TEST_PIN_NAMES[t.pin] << " is used more than once. "
+            << "This is a problem with the timerHardware array (in target.c). "
+            << "Check the array for typos. Then check the size of the array "
+            << "initializers; it must be USABLE_TIMER_CHANNEL_COUNT.";
+    EXPECT_EQ(USABLE_TIMER_CHANNEL_COUNT, usedPins.size());
+}
+
+namespace {
+std::string writeUsedTimers(const std::bitset<TEST_TIMER_SIZE> &tset) {
+    std::stringstream used_timers;
+    if (tset.any()) {
+        unsigned int timer{0};
+        for (; timer < TEST_TIMER_SIZE; ++timer)
+            if (tset[timer]) {
+                used_timers << "( TIM_N(" << timer << ')';
+                break;
+            }
+        for (++timer; timer < TEST_TIMER_SIZE; ++timer)
+            if (tset[timer]) used_timers << " | TIM_N(" << timer << ')';
+        used_timers << " )";
+    } else {
+        used_timers << "(0)";
+    }
+    return used_timers.str();
+}
+}
+
+TEST(TimerDefinitionTest, Test_usedTimers)
+{
+    std::bitset<TEST_TIMER_SIZE> expected;
+    for (const timerHardware_t &t : timerHardware)
+        expected |= TIM_N(t.timer);
+    const std::bitset<TEST_TIMER_SIZE> actual{USED_TIMERS};
+    EXPECT_EQ(expected, actual)
+        << "Used timers mismatch. target.c says " << expected << ", but "
+        << "target.h says " << actual << ". This has two possible causes: "
+        << "(1) The USED_TIMERS bitmap (in target.h) is outdated and out of "
+        << "sync with timerHardware (in target.c). (2) There is an "
+        << "inconsistency between USABLE_TIMER_CHANNEL_COUNT and the length "
+        << "of timerHardware's initializer list.";
+    std::cerr
+        << "USED_TIMERS definition based on timerHardware:" << std::endl
+        << writeUsedTimers(expected) << std::endl;
+}
+
+// STUBS
+
+extern "C" {
+    void spiPinConfigure(int) {}
+    int spiPinConfig(int) { return 0; }
+    void spiInit(int) {}
+
+    int i2cConfig(int) { return 0; }
+    void i2cHardwareConfigure(int) {}
+    void i2cInit(int) {}
+
+    void bstInit(int) {}
+}

src/test/unit/timer_definition_unittest.include/drivers/timer.h

@@ -0,0 +1,21 @@
+#include <mock_enums.h>
+
+typedef struct timerHardware_s {
+    enum TestTimerEnum timer;
+    enum TestChannelEnum channel;
+    enum TestPinEnum pin;
+    enum TestTimUseEnum purpose;
+    unsigned int def_tim_counter;
+} timerHardware_t;
+
+// F7 and F4 have 6 arguments, F3 and F1 have 5 arguments.
+#define DEF_TIM(timer_, channel_, pin_, purpose_, ...)  \
+{                                                       \
+    .timer = timer_,                                    \
+    .channel = channel_,                                \
+    .pin = pin_,                                        \
+    .purpose = purpose_,                                \
+    .def_tim_counter = __COUNTER__,                     \
+}
+
+#define TIM_N(n) (1 << (n))

src/test/unit/timer_definition_unittest.include/mock_enums.h

@@ -0,0 +1,66 @@
+#pragma once
+
+enum TestTimerEnum {
+    TIM0, TIM1, TIM2, TIM3, TIM4, TIM5, TIM6, TIM7, TIM8, TIM9,
+    TIM10, TIM11, TIM12, TIM13, TIM14, TIM15, TIM16, TIM17, TIM18, TIM19, TIM20,
+    TEST_TIMER_SIZE,
+};
+
+enum TestChannelEnum {
+    CH0, CH1, CH2, CH3, CH4, CH5, CH6, CH7, CH9, CH10, CH1N, CH2N, CH3N,
+    TEST_CHANNEL_SIZE,
+};
+
+// Keep this in sync with TEST_PIN_NAMES below.
+enum TestPinEnum {
+    PA0, PA1, PA2, PA3, PA4, PA5, PA6, PA7, PA8, PA9,
+    PA10, PA11, PA12, PA13, PA14, PA15,
+    PB0, PB1, PB2, PB3, PB4, PB5, PB6, PB7, PB8, PB9,
+    PB10, PB11, PB12, PB13, PB14, PB15,
+    PC0, PC1, PC2, PC3, PC4, PC5, PC6, PC7, PC8, PC9,
+    PC10, PC11, PC12, PC13, PC14, PC15,
+    PD0, PD1, PD2, PD3, PD4, PD5, PD6, PD7, PD8, PD9,
+    PD10, PD11, PD12, PD13, PD14, PD15,
+    PE0, PE1, PE2, PE3, PE4, PE5, PE6, PE7, PE8, PE9,
+    PE10, PE11, PE12, PE13, PE14, PE15,
+    PF0, PF1, PF2, PF3, PF4, PF5, PF6, PF7, PF8, PF9,
+    PF10, PF11, PF12, PF13, PF14, PF15,
+    PG0, PG1, PG2, PG3, PG4, PG5, PG6, PG7, PG8, PG9,
+    PG10, PG11, PG12, PG13, PG14, PG15,
+    PH0, PH1, PH2, PH3, PH4, PH5, PH6, PH7, PH8, PH9,
+    PH10, PH11, PH12, PH13, PH14, PH15, TEST_PIN_SIZE,
+};
+
+// Keep this in sync with TestPinEnum above.
+const char *const TEST_PIN_NAMES[TEST_PIN_SIZE] = {
+    "PA0", "PA1", "PA2", "PA3", "PA4", "PA5", "PA6", "PA7", "PA8", "PA9",
+    "PA10", "PA11", "PA12", "PA13", "PA14", "PA15",
+    "PB0", "PB1", "PB2", "PB3", "PB4", "PB5", "PB6", "PB7", "PB8", "PB9",
+    "PB10", "PB11", "PB12", "PB13", "PB14", "PB15",
+    "PC0", "PC1", "PC2", "PC3", "PC4", "PC5", "PC6", "PC7", "PC8", "PC9",
+    "PC10", "PC11", "PC12", "PC13", "PC14", "PC15",
+    "PD0", "PD1", "PD2", "PD3", "PD4", "PD5", "PD6", "PD7", "PD8", "PD9",
+    "PD10", "PD11", "PD12", "PD13", "PD14", "PD15",
+    "PE0", "PE1", "PE2", "PE3", "PE4", "PE5", "PE6", "PE7", "PE8", "PE9",
+    "PE10", "PE11", "PE12", "PE13", "PE14", "PE15",
+    "PF0", "PF1", "PF2", "PF3", "PF4", "PF5", "PF6", "PF7", "PF8", "PF9",
+    "PF10", "PF11", "PF12", "PF13", "PF14", "PF15",
+    "PG0", "PG1", "PG2", "PG3", "PG4", "PG5", "PG6", "PG7", "PG8", "PG9",
+    "PG10", "PG11", "PG12", "PG13", "PG14", "PG15",
+    "PH0", "PH1", "PH2", "PH3", "PH4", "PH5", "PH6", "PH7", "PH8", "PH9",
+    "PH10", "PH11", "PH12", "PH13", "PH14", "PH15",
+};
+
+enum TestTimUseEnum {
+    TIM_USE_ANY,
+    TIM_USE_BEEPER,
+    TIM_USE_CAMERA_CONTROL,
+    TIM_USE_LED,
+    TIM_USE_MOTOR,
+    TIM_USE_NONE,
+    TIM_USE_PPM,
+    TIM_USE_PWM,
+    TIM_USE_SERVO,
+    TIM_USE_TRANSPONDER,
+    TEST_TIM_USE_SIZE,
+};

src/test/unit/timer_definition_unittest.include/pg/bus_bst.h

@@ -0,0 +1 @@
+void bstInit(int);

src/test/unit/timer_definition_unittest.include/pg/bus_i2c.h

@@ -0,0 +1,5 @@
+#define I2CDEV_2 (1)
+
+int i2cConfig(int);
+void i2cHardwareConfigure(int);
+void i2cInit(int);

src/test/unit/timer_definition_unittest.include/pg/bus_spi.h

@@ -0,0 +1,5 @@
+#define SPIDEV_1 (0)
+
+void spiPinConfigure(int);
+int spiPinConfig(int);
+void spiInit(int);

src/test/unit/timer_definition_unittest.include/platform.h

@@ -0,0 +1,5 @@
+#include <mock_enums.h>
+#include "target/common_pre.h"
+#include "target.h"
+#include "target/common_post.h"
+#include "target/common_defaults_post.h"