From 497b123958619387ddb2c4eacabe57f7ab053487 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 26 Jun 2018 12:39:27 +0800 Subject: [PATCH 1/7] =?UTF-8?q?unit-test-app:=20don=E2=80=99t=20include=20?= =?UTF-8?q?project.mk=20for=20ut-=20targets?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If project.mk is included twice in recursive invocation of Make, some variables defined on the first pass will not be redefined on the second pass. Rather than cleaning up these variables before calling Make recursively, don’t include IDF project.mk at all, if one of the ut- targets is requested. --- tools/unit-test-app/Makefile | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/tools/unit-test-app/Makefile b/tools/unit-test-app/Makefile index a7b3c45ff4..222cfb1361 100644 --- a/tools/unit-test-app/Makefile +++ b/tools/unit-test-app/Makefile @@ -5,9 +5,10 @@ PROJECT_NAME := unit-test-app -include $(IDF_PATH)/make/project.mk - ifeq ($(MAKELEVEL),0) +# Set default target +all: + # Define helper targets only when not recursing # List of unit-test-app configurations. @@ -22,8 +23,9 @@ CONFIG_CLEAN_TARGETS := $(addprefix ut-clean-,$(CONFIG_NAMES)) CONFIG_APPLY_TARGETS := $(addprefix ut-apply-config-,$(CONFIG_NAMES)) # Build (intermediate) and output (artifact) directories -BUILDS_DIR := $(PROJECT_PATH)/builds -BINARIES_DIR := $(PROJECT_PATH)/output +PROJECT_DIR := $(abspath $(dir $(firstword $(MAKEFILE_LIST)))) +BUILDS_DIR := $(PROJECT_DIR)/builds +BINARIES_DIR := $(PROJECT_DIR)/output # This generates per-config targets (clean, build, apply-config). define GenerateConfigTargets @@ -57,14 +59,14 @@ $(BINARIES_DIR)/%/$(PROJECT_NAME).bin: configs/% mkdir -p $(BINARIES_DIR)/$*/bootloader mkdir -p $(BUILDS_DIR)/$* # Prepare configuration: top-level sdkconfig.defaults file plus the current configuration (configs/$*) - $(summary) CONFIG $(BUILDS_DIR)/$*/sdkconfig + echo CONFIG $(BUILDS_DIR)/$*/sdkconfig rm -f $(BUILDS_DIR)/$*/sdkconfig cat sdkconfig.defaults > $(BUILDS_DIR)/$*/sdkconfig.defaults echo "" >> $(BUILDS_DIR)/$*/sdkconfig.defaults # in case there is no trailing newline in sdkconfig.defaults cat configs/$* >> $(BUILDS_DIR)/$*/sdkconfig.defaults # Build, tweaking paths to sdkconfig and sdkconfig.defaults - $(summary) BUILD_CONFIG $(BUILDS_DIR)/$* + echo BUILD_CONFIG $(BUILDS_DIR)/$* # 'TEST_COMPONENTS=names' option can be added to configs/$* to limit the set # of tests to build for given configuration. # Build all tests if this option is not present. @@ -78,7 +80,7 @@ $(BINARIES_DIR)/%/$(PROJECT_NAME).bin: configs/% TEST_COMPONENTS="$${test_components}" \ TESTS_ALL=$${tests_all} \ EXCLUDE_COMPONENTS="$${exclude_components}" - $(MAKE) print_flash_cmd \ + $(MAKE) --silent print_flash_cmd \ BUILD_DIR_BASE=$(BUILDS_DIR)/$* \ SDKCONFIG=$(BUILDS_DIR)/$*/sdkconfig \ | sed -e 's:'$(BUILDS_DIR)/$*/'::g' \ @@ -113,17 +115,29 @@ ut-help: help: ut-help -.PHONY: ut-build-all-configs ut-clean-all-configs \ - $(CONFIG_BUILD_TARGETS) $(CONFIG_CLEAN_TARGETS) $(CONFIG_APPLY_TARGETS) \ +LOCAL_TARGETS := ut-build-all-configs ut-clean-all-configs \ + $(CONFIG_BUILD_TARGETS) $(CONFIG_CLEAN_TARGETS) \ ut-help +.PHONY: $(LOCAL_TARGETS) + NON_INTERACTIVE_TARGET += ut-apply-config-% ut-clean-% ut-build-% \ ut-build-all-configs ut-clean-all-configs -else # MAKELEVEL == 0 +endif # MAKELEVEL == 0 + + +# When targets defined in this makefile are built, don't need to include the main project makefile. +# This prevents some variables which depend on build directory from being set erroneously. +ifeq ($(filter $(LOCAL_TARGETS),$(MAKECMDGOALS)),) + +include $(IDF_PATH)/make/project.mk + +endif # If recursing, print the actual list of tests being built +ifneq ($(MAKELEVEL),0) $(info TESTS $(foreach comp,$(TEST_COMPONENT_NAMES),$(patsubst %_test,%,$(comp)))) -endif # MAKELEVEL == 0 +endif # MAKELEVEL != 0 From 3fcc3689ce35046220b15b592801145ad4c49760 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 26 Jun 2018 14:37:36 +0800 Subject: [PATCH 2/7] spi_master, ulp: fix aliasing errors in unit tests --- components/driver/test/test_spi_master.c | 2 +- components/ulp/test/test_ulp.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/driver/test/test_spi_master.c b/components/driver/test/test_spi_master.c index 8814088f6d..5a7b490109 100644 --- a/components/driver/test/test_spi_master.c +++ b/components/driver/test/test_spi_master.c @@ -759,7 +759,7 @@ static void task_slave(void* arg) do { TEST_ESP_OK( spi_slave_transmit( context->spi, &t, portMAX_DELAY ) ); } while ( t.trans_len == 0 ); - *(uint32_t*)recvbuf = t.trans_len; + memcpy(recvbuf, &t.trans_len, sizeof(uint32_t)); *(uint8_t**)(recvbuf+4) = txdata.start; ESP_LOGI( SLAVE_TAG, "received: %d", t.trans_len ); xRingbufferSend( ringbuf, recvbuf, 8+(t.trans_len+7)/8, portMAX_DELAY ); diff --git a/components/ulp/test/test_ulp.c b/components/ulp/test/test_ulp.c index 197a12b61b..766a71742e 100644 --- a/components/ulp/test/test_ulp.c +++ b/components/ulp/test/test_ulp.c @@ -271,7 +271,7 @@ TEST_CASE("ulp power consumption in deep sleep", "[ulp][ignore]") { assert(CONFIG_ULP_COPROC_RESERVE_MEM >= 4 && "this test needs ULP_COPROC_RESERVE_MEM option set in menuconfig"); ulp_insn_t insn = I_HALT(); - RTC_SLOW_MEM[0] = *(uint32_t*) &insn; + memcpy(&RTC_SLOW_MEM[0], &insn, sizeof(insn)); REG_WRITE(SENS_ULP_CP_SLEEP_CYC0_REG, 0x8000); From cfde037f9a2878e3db973e8c4afaad0545fdc9b2 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 27 Jun 2018 14:46:20 +0800 Subject: [PATCH 3/7] heap: fix unit test for the case when less than 10k of IRAM is available --- components/heap/test/test_malloc_caps.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/components/heap/test/test_malloc_caps.c b/components/heap/test/test_malloc_caps.c index 0be5f6a6ca..950f18455c 100644 --- a/components/heap/test/test_malloc_caps.c +++ b/components/heap/test/test_malloc_caps.c @@ -9,6 +9,7 @@ #include "esp_heap_caps.h" #include "esp_spi_flash.h" #include +#include TEST_CASE("Capabilities allocator test", "[heap]") { @@ -38,18 +39,24 @@ TEST_CASE("Capabilities allocator test", "[heap]") TEST_ASSERT((((int)m1)&0xFF000000)==0x3F000000); free(m1); - printf("Freeing; allocating 10K of 32K-capable RAM\n"); - m1 = heap_caps_malloc(10*1024, MALLOC_CAP_32BIT); + //The goal here is to allocate from IRAM. Since there is no external IRAM (yet) + //the following gives size of IRAM-only (not D/IRAM) memory. + size_t free_iram = heap_caps_get_free_size(MALLOC_CAP_INTERNAL) - + heap_caps_get_free_size(MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL); + size_t alloc32 = MIN(free_iram / 2, 10*1024) & (~3); + printf("Freeing; allocating %u bytes of 32K-capable RAM\n", alloc32); + m1 = heap_caps_malloc(alloc32, MALLOC_CAP_32BIT); printf("--> %p\n", m1); + //Check that we got IRAM back + TEST_ASSERT((((int)m1)&0xFF000000)==0x40000000); free8 = heap_caps_get_free_size(MALLOC_CAP_8BIT); free32 = heap_caps_get_free_size(MALLOC_CAP_32BIT); printf("Free 8bit-capable memory (after 32-bit): %dK, 32-bit capable memory %dK\n", free8, free32); - //Only 32-bit should have gone down by 10K: 32-bit isn't necessarily 8bit capable - TEST_ASSERT(free32<(free32start-10*1024)); + //Only 32-bit should have gone down by alloc32: 32-bit isn't necessarily 8bit capable + TEST_ASSERT(free32<(free32start-alloc32)); TEST_ASSERT(free8==free8start); - //Assume we got IRAM back - TEST_ASSERT((((int)m1)&0xFF000000)==0x40000000); free(m1); + printf("Allocating impossible caps\n"); m1= heap_caps_malloc(10*1024, MALLOC_CAP_8BIT|MALLOC_CAP_EXEC); printf("--> %p\n", m1); @@ -57,14 +64,15 @@ TEST_CASE("Capabilities allocator test", "[heap]") printf("Testing changeover iram -> dram"); // priorities will exhaust IRAM first, then start allocating from DRAM for (x=0; x<10; x++) { - m2[x]= heap_caps_malloc(10*1024, MALLOC_CAP_32BIT); + m2[x]= heap_caps_malloc(alloc32, MALLOC_CAP_32BIT); printf("--> %p\n", m2[x]); } TEST_ASSERT((((int)m2[0])&0xFF000000)==0x40000000); TEST_ASSERT((((int)m2[9])&0xFF000000)==0x3F000000); printf("Test if allocating executable code still gives IRAM, even with dedicated IRAM region depleted\n"); // (the allocation should come from D/IRAM) - m1= heap_caps_malloc(10*1024, MALLOC_CAP_EXEC); + free_iram = heap_caps_get_free_size(MALLOC_CAP_EXEC); + m1= heap_caps_malloc(MIN(free_iram / 2, 10*1024), MALLOC_CAP_EXEC); printf("--> %p\n", m1); TEST_ASSERT((((int)m1)&0xFF000000)==0x40000000); free(m1); From a1dadda7d6ac097b21a74ec57fdfcb9b09c739c7 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 27 Jun 2018 14:47:05 +0800 Subject: [PATCH 4/7] newlib: fix unit test for psram config --- components/newlib/test/test_newlib.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/components/newlib/test/test_newlib.c b/components/newlib/test/test_newlib.c index 2836fc3d27..10d81e1630 100644 --- a/components/newlib/test/test_newlib.c +++ b/components/newlib/test/test_newlib.c @@ -124,15 +124,20 @@ static bool fn_in_rom(void *fn, const char *name) TEST_CASE("check if ROM or Flash is used for functions", "[newlib]") { -#ifdef CONFIG_NEWLIB_NANO_FORMAT +#if defined(CONFIG_NEWLIB_NANO_FORMAT) && !defined(CONFIG_SPIRAM_SUPPORT) TEST_ASSERT(fn_in_rom(printf, "printf")); TEST_ASSERT(fn_in_rom(sscanf, "sscanf")); #else TEST_ASSERT_FALSE(fn_in_rom(printf, "printf")); TEST_ASSERT_FALSE(fn_in_rom(sscanf, "sscanf")); #endif +#if !defined(CONFIG_SPIRAM_SUPPORT) TEST_ASSERT(fn_in_rom(atoi, "atoi")); TEST_ASSERT(fn_in_rom(strtol, "strtol")); +#else + TEST_ASSERT_FALSE(fn_in_rom(atoi, "atoi")); + TEST_ASSERT_FALSE(fn_in_rom(strtol, "strtol")); +#endif } #ifndef CONFIG_NEWLIB_NANO_FORMAT From f81e71f622a4fc299278c112795d0973c3f43159 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 27 Jun 2018 14:47:31 +0800 Subject: [PATCH 5/7] freertos: bump limit for spinlock performance test to 300 cycles --- components/idf_test/include/idf_performance.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/idf_test/include/idf_performance.h b/components/idf_test/include/idf_performance.h index f98f75c317..fc128eecbb 100644 --- a/components/idf_test/include/idf_performance.h +++ b/components/idf_test/include/idf_performance.h @@ -11,7 +11,7 @@ /* declare the performance here */ #define IDF_PERFORMANCE_MAX_HTTPS_REQUEST_BIN_SIZE 800 #define IDF_PERFORMANCE_MAX_FREERTOS_SPINLOCK_CYCLES_PER_OP 200 -#define IDF_PERFORMANCE_MAX_FREERTOS_SPINLOCK_CYCLES_PER_OP_PSRAM 270 +#define IDF_PERFORMANCE_MAX_FREERTOS_SPINLOCK_CYCLES_PER_OP_PSRAM 300 #define IDF_PERFORMANCE_MAX_FREERTOS_SPINLOCK_CYCLES_PER_OP_UNICORE 130 #define IDF_PERFORMANCE_MAX_ESP_TIMER_GET_TIME_PER_CALL 1000 #define IDF_PERFORMANCE_MAX_SPI_PER_TRANS_NO_POLLING 30 From 3c359763da2f62d72b53b5814ccc89fc3371b832 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 27 Jun 2018 17:16:38 +0800 Subject: [PATCH 6/7] heap: move get_all_caps to IRAM, used in unit test --- components/heap/heap_private.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/heap/heap_private.h b/components/heap/heap_private.h index 5103cfd178..a8a0ac9fda 100644 --- a/components/heap/heap_private.h +++ b/components/heap/heap_private.h @@ -49,7 +49,7 @@ extern SLIST_HEAD(registered_heap_ll, heap_t_) registered_heaps; bool heap_caps_match(const heap_t *heap, uint32_t caps); /* return all possible capabilities (across all priorities) for a given heap */ -inline static uint32_t get_all_caps(const heap_t *heap) +inline static IRAM_ATTR uint32_t get_all_caps(const heap_t *heap) { if (heap->heap == NULL) { return 0; From abbc13af622aadc30b8924c38e7277a21502f8ac Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Mon, 30 Jul 2018 14:24:03 +1000 Subject: [PATCH 7/7] freertos: Remove either one or two assertions from "fast path" of vPortCPUReleaseMutex() Saves a few cycles by only testing the count validity once, and never for the common case where the mutex was not recursively locked. --- components/freertos/portmux_impl.inc.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/components/freertos/portmux_impl.inc.h b/components/freertos/portmux_impl.inc.h index 29bfbfc346..07a7ce9fe1 100644 --- a/components/freertos/portmux_impl.inc.h +++ b/components/freertos/portmux_impl.inc.h @@ -155,17 +155,15 @@ static inline void PORTMUX_RELEASE_MUX_FN_NAME(portMUX_TYPE *mux) { #endif assert(coreID == mux->owner); // This is a mutex we didn't lock, or it's corrupt - assert(mux->count > 0); // Indicates memory corruption - assert(mux->count < 0x100); // Indicates memory corruption mux->count--; if(mux->count == 0) { mux->owner = portMUX_FREE_VAL; - } + } else { + assert(mux->count < 0x100); // Indicates memory corruption #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG_RECURSIVE - else { ets_printf("Recursive unlock: count=%d last locked %s line %d, curr %s line %d\n", mux->count, lastLockedFn, lastLockedLine, fnName, line); - } #endif + } #endif //!CONFIG_FREERTOS_UNICORE }