Merge branch 'bugfix/extra_cflags' into 'master'

cmake: add EXTRA_CFLAGS and clean warnings

See merge request idf/esp-idf!3805

.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

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();

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]")

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]")
 {

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() );

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.
 */

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);

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

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

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"

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

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}")