From ac18b1024a9934cfdec844b2283924743b94b4e9 Mon Sep 17 00:00:00 2001 From: Anton Maklakov Date: Tue, 27 Nov 2018 22:46:50 +0800 Subject: [PATCH 1/3] cmake: Add support for EXTRA_CFLAGS and EXTRA_CXXFLAGS --- tools/cmake/idf_functions.cmake | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tools/cmake/idf_functions.cmake b/tools/cmake/idf_functions.cmake index 82b00bbbac..e8cd828325 100644 --- a/tools/cmake/idf_functions.cmake +++ b/tools/cmake/idf_functions.cmake @@ -164,6 +164,20 @@ function(idf_set_global_compile_options) # go into the final binary so have no impact on size) list(APPEND compile_options "-ggdb") + # Use EXTRA_CFLAGS, EXTRA_CXXFLAGS and EXTRA_CPPFLAGS to add more priority options to the compiler + # EXTRA_CPPFLAGS is used for both C and C++ + # Unlike environments' CFLAGS/CXXFLAGS/CPPFLAGS which work for both host and target build, + # these works only for target build + set(EXTRA_CFLAGS "$ENV{EXTRA_CFLAGS}") + set(EXTRA_CXXFLAGS "$ENV{EXTRA_CXXFLAGS}") + set(EXTRA_CPPFLAGS "$ENV{EXTRA_CPPFLAGS}") + spaces2list(EXTRA_CFLAGS) + spaces2list(EXTRA_CXXFLAGS) + spaces2list(EXTRA_CPPFLAGS) + list(APPEND c_compile_options ${EXTRA_CFLAGS}) + list(APPEND cxx_compile_options ${EXTRA_CXXFLAGS}) + list(APPEND compile_options ${EXTRA_CPPFLAGS}) + # Temporary trick to support both gcc5 and gcc8 builds list(APPEND compile_definitions "GCC_NOT_5_2_0=${GCC_NOT_5_2_0}") From 0652a4b71456cfe72abac1d4d80fb4eaec23b060 Mon Sep 17 00:00:00 2001 From: Anton Maklakov Date: Tue, 27 Nov 2018 19:16:18 +0800 Subject: [PATCH 2/3] ci: Use more pedantic checking for examples and ut --- .gitlab-ci.yml | 10 ++++------ tools/ci/build_examples.sh | 2 +- tools/ci/build_examples_cmake.sh | 2 +- tools/ci/configure_ci_environment.sh | 4 ++++ tools/ci/test_configure_ci_environment.sh | 6 +++++- 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b778bed88d..294fe3b058 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -200,15 +200,15 @@ build_ssc_02: build_esp_idf_tests_make: <<: *build_esp_idf_unit_test_template script: - - export EXTRA_CFLAGS="-Werror -Werror=deprecated-declarations" + - export EXTRA_CFLAGS=${PEDANTIC_CFLAGS} - export EXTRA_CXXFLAGS=${EXTRA_CFLAGS} - cd $CI_PROJECT_DIR/tools/unit-test-app - MAKEFLAGS= make help # make sure kconfig tools are built in single process - make ut-clean-all-configs - make ut-build-all-configs - python tools/UnitTestParser.py + # Check if the tests demand Make built binaries. If not, delete them - if [ "$UNIT_TEST_BUILD_SYSTEM" == "make" ]; then exit 0; fi - # If Make, delete the CMake built artifacts - rm -rf builds output sdkconfig - rm -rf components/idf_test/unit_test/TestCaseAll.yml - rm -rf components/idf_test/unit_test/CIConfigs/*.yml @@ -217,16 +217,14 @@ build_esp_idf_tests_cmake: <<: *build_esp_idf_unit_test_template script: - export PATH="$IDF_PATH/tools:$PATH" - - export EXTRA_CFLAGS="-Werror -Werror=deprecated-declarations" + - export EXTRA_CFLAGS=${PEDANTIC_CFLAGS} - export EXTRA_CXXFLAGS=${EXTRA_CFLAGS} - cd $CI_PROJECT_DIR/tools/unit-test-app - # Build with CMake first - idf.py ut-clean-all-configs - idf.py ut-build-all-configs - python tools/UnitTestParser.py - # Check if test demands CMake or Make built binaries. If CMake leave the built artifacts as is then exit. + # Check if the tests demand CMake built binaries. If not, delete them - if [ "$UNIT_TEST_BUILD_SYSTEM" == "cmake" ]; then exit 0; fi - # If Make, delete the CMake built artifacts - rm -rf builds output sdkconfig - rm -rf components/idf_test/unit_test/TestCaseAll.yml - rm -rf components/idf_test/unit_test/CIConfigs/*.yml diff --git a/tools/ci/build_examples.sh b/tools/ci/build_examples.sh index 9ab8fee578..eb8a5993ae 100755 --- a/tools/ci/build_examples.sh +++ b/tools/ci/build_examples.sh @@ -129,7 +129,7 @@ build_example () { local EXAMPLE_DIR_REL=${EXAMPLE_DIR#"${COPY_ROOT_PARENT}"} pushd "example_builds/${ID}/${EXAMPLE_DIR_REL}" # be stricter in the CI build than the default IDF settings - export EXTRA_CFLAGS="-Werror -Werror=deprecated-declarations" + export EXTRA_CFLAGS=${PEDANTIC_CFLAGS} export EXTRA_CXXFLAGS=${EXTRA_CFLAGS} # build non-verbose first diff --git a/tools/ci/build_examples_cmake.sh b/tools/ci/build_examples_cmake.sh index 8686219814..009cc22059 100755 --- a/tools/ci/build_examples_cmake.sh +++ b/tools/ci/build_examples_cmake.sh @@ -119,7 +119,7 @@ build_example () { cp -r "${EXAMPLE_DIR}" "example_builds/${ID}" pushd "example_builds/${ID}/${EXAMPLE_NAME}" # be stricter in the CI build than the default IDF settings - export EXTRA_CFLAGS="-Werror -Werror=deprecated-declarations" + export EXTRA_CFLAGS=${PEDANTIC_CFLAGS} export EXTRA_CXXFLAGS=${EXTRA_CFLAGS} # build non-verbose first diff --git a/tools/ci/configure_ci_environment.sh b/tools/ci/configure_ci_environment.sh index a92600e92a..1d4380ad56 100644 --- a/tools/ci/configure_ci_environment.sh +++ b/tools/ci/configure_ci_environment.sh @@ -30,3 +30,7 @@ else export IS_PUBLIC= fi unset REF + +# Compiler flags to thoroughly check the IDF code in some CI jobs +# (Depends on default options '-Wno-error=XXX' used in the IDF build system) +export PEDANTIC_CFLAGS="-Werror -Werror=deprecated-declarations -Werror=unused-variable -Werror=unused-but-set-variable -Werror=unused-function" diff --git a/tools/ci/test_configure_ci_environment.sh b/tools/ci/test_configure_ci_environment.sh index 1ad5f430ca..8f513de95d 100755 --- a/tools/ci/test_configure_ci_environment.sh +++ b/tools/ci/test_configure_ci_environment.sh @@ -30,5 +30,9 @@ assert_branch_public v1.2-rc77 1 assert_branch_public v1.2.3-rc1 1 assert_branch_public v1.2.3invalid -rm -f .gitmodules +( + . ./configure_ci_environment.sh + [[ $PEDANTIC_CFLAGS ]] || { echo "PEDANTIC_CFLAGS is not defined"; exit 1; } +) || { exit 1; } +rm -f .gitmodules From 81bf07ed4d28ed879df873e4ada885fabd6fb291 Mon Sep 17 00:00:00 2001 From: Anton Maklakov Date: Wed, 5 Dec 2018 20:22:25 +0800 Subject: [PATCH 3/3] test: Fix some unused identifier warnings --- components/driver/test/test_i2c.c | 18 ------------------ components/esp32/test/test_4mpsram.c | 3 +++ components/esp32/test/test_sleep.c | 4 +++- .../esp32/test/test_wifi_lib_git_commit.c | 2 -- .../freertos/test/test_suspend_scheduler.c | 3 ++- components/freertos/test/test_tasks_snapshot.c | 7 +++++-- 6 files changed, 13 insertions(+), 24 deletions(-) diff --git a/components/driver/test/test_i2c.c b/components/driver/test/test_i2c.c index 07432b002b..48aea2a4b7 100644 --- a/components/driver/test/test_i2c.c +++ b/components/driver/test/test_i2c.c @@ -51,24 +51,6 @@ static DRAM_ATTR i2c_dev_t *const I2C[I2C_NUM_MAX] = { &I2C0, &I2C1 }; -static esp_err_t i2c_master_read_slave(i2c_port_t i2c_num, uint8_t *data_rd, size_t size) -{ - if (size == 0) { - return ESP_OK; - } - i2c_cmd_handle_t cmd = i2c_cmd_link_create(); - i2c_master_start(cmd); - i2c_master_write_byte(cmd, ( ESP_SLAVE_ADDR << 1 ) | READ_BIT, ACK_CHECK_EN); - if (size > 1) { - i2c_master_read(cmd, data_rd, size - 1, ACK_VAL); - } - i2c_master_read_byte(cmd, data_rd + size - 1, NACK_VAL); - i2c_master_stop(cmd); - esp_err_t ret = i2c_master_cmd_begin(i2c_num, cmd, 1000 / portTICK_RATE_MS); - i2c_cmd_link_delete(cmd); - return ret; -} - static esp_err_t i2c_master_write_slave(i2c_port_t i2c_num, uint8_t *data_wr, size_t size) { i2c_cmd_handle_t cmd = i2c_cmd_link_create(); diff --git a/components/esp32/test/test_4mpsram.c b/components/esp32/test/test_4mpsram.c index 26e2c8d5e4..a1cd36023c 100644 --- a/components/esp32/test/test_4mpsram.c +++ b/components/esp32/test/test_4mpsram.c @@ -2,9 +2,11 @@ #include "unity.h" #include "esp_log.h" #include "driver/spi_common.h" +#include "sdkconfig.h" static const char TAG[] = "test_psram"; +#ifdef CONFIG_SPIRAM_SUPPORT static void test_psram_content() { const int test_size = 2048; @@ -32,6 +34,7 @@ static void test_psram_content() free(test_area); } +#endif // NOTE: this unit test rely on the config that PSRAM of 8MB is used only when CONFIG_SPIRAM_BNKSWITCH_ENABLE is set TEST_CASE("can use spi when not being used by psram", "[psram_4m]") diff --git a/components/esp32/test/test_sleep.c b/components/esp32/test/test_sleep.c index f36e759845..d2e2d9b279 100644 --- a/components/esp32/test/test_sleep.c +++ b/components/esp32/test/test_sleep.c @@ -17,13 +17,14 @@ #include "rom/rtc.h" #include "esp_newlib.h" #include "test_utils.h" +#include "sdkconfig.h" #define ESP_EXT0_WAKEUP_LEVEL_LOW 0 #define ESP_EXT0_WAKEUP_LEVEL_HIGH 1 static struct timeval tv_start, tv_stop; - +#ifndef CONFIG_FREERTOS_UNICORE static void deep_sleep_task(void *arg) { esp_deep_sleep_start(); @@ -40,6 +41,7 @@ static void do_deep_sleep_from_app_cpu() ; } } +#endif TEST_CASE("wake up from deep sleep using timer", "[deepsleep][reset=DEEPSLEEP_RESET]") { diff --git a/components/esp32/test/test_wifi_lib_git_commit.c b/components/esp32/test/test_wifi_lib_git_commit.c index 8b5633f9b1..fbf1dea0e5 100644 --- a/components/esp32/test/test_wifi_lib_git_commit.c +++ b/components/esp32/test/test_wifi_lib_git_commit.c @@ -5,8 +5,6 @@ #include "esp_log.h" #include "esp_wifi_internal.h" -static const char* TAG = "test_wifi_lib_git_commit_id"; - TEST_CASE("wifi lib git commit id","[wifi]") { TEST_ESP_OK( esp_wifi_internal_git_commit_id_check() ); diff --git a/components/freertos/test/test_suspend_scheduler.c b/components/freertos/test/test_suspend_scheduler.c index 1599ef685c..9aef2ce7ec 100644 --- a/components/freertos/test/test_suspend_scheduler.c +++ b/components/freertos/test/test_suspend_scheduler.c @@ -11,6 +11,7 @@ #include "test_utils.h" #include "driver/timer.h" +#include "sdkconfig.h" static SemaphoreHandle_t isr_semaphore; static volatile unsigned isr_count; @@ -194,6 +195,7 @@ TEST_CASE("Scheduler disabled can wake multiple tasks on resume", "[freertos]") } } +#ifndef CONFIG_FREERTOS_UNICORE static volatile bool sched_suspended; static void suspend_scheduler_5ms_task_fn(void *ignore) { @@ -207,7 +209,6 @@ static void suspend_scheduler_5ms_task_fn(void *ignore) vTaskDelete(NULL); } -#ifndef CONFIG_FREERTOS_UNICORE /* If the scheduler is disabled on one CPU (A) with a task blocked on something, and a task on B (where scheduler is running) wakes it, then the task on A should be woken on resume. */ diff --git a/components/freertos/test/test_tasks_snapshot.c b/components/freertos/test/test_tasks_snapshot.c index c9364cc182..bc42e1a90c 100644 --- a/components/freertos/test/test_tasks_snapshot.c +++ b/components/freertos/test/test_tasks_snapshot.c @@ -7,6 +7,7 @@ #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "unity.h" +#include "sdkconfig.h" #define TEST_MAX_TASKS_NUM 32 @@ -15,16 +16,18 @@ TEST_CASE("Tasks snapshot", "[freertos]") { TaskSnapshot_t tasks[TEST_MAX_TASKS_NUM]; UBaseType_t tcb_sz; +#ifndef CONFIG_FREERTOS_UNICORE int other_core_id = xPortGetCoreID() == 0 ? 1 : 0; +#endif // uxTaskGetSnapshotAll is supposed to be called when all tasks on both CPUs are // inactive and can not alter FreeRTOS internal tasks lists, e.g. from panic handler unsigned state = portENTER_CRITICAL_NESTED(); -#if CONFIG_FREERTOS_UNICORE == 0 +#ifndef CONFIG_FREERTOS_UNICORE esp_cpu_stall(other_core_id); #endif UBaseType_t task_num = uxTaskGetSnapshotAll(tasks, TEST_MAX_TASKS_NUM, &tcb_sz); -#if CONFIG_FREERTOS_UNICORE == 0 +#ifndef CONFIG_FREERTOS_UNICORE esp_cpu_unstall(other_core_id); #endif portEXIT_CRITICAL_NESTED(state);