From 81f2a94f9a76da695fc7e8948eaf509b514fffec Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Fri, 15 Apr 2022 10:51:07 +0200 Subject: [PATCH 1/6] esp_system: make dependencies on vfs and esp_wifi optional --- components/esp_system/CMakeLists.txt | 8 ++++++++ components/esp_system/startup.c | 9 +++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/components/esp_system/CMakeLists.txt b/components/esp_system/CMakeLists.txt index d92be939e6..5bd2af9ab6 100644 --- a/components/esp_system/CMakeLists.txt +++ b/components/esp_system/CMakeLists.txt @@ -95,3 +95,11 @@ idf_component_optional_requires(PRIVATE app_update) if(CONFIG_PM_ENABLE) idf_component_optional_requires(PRIVATE pm) endif() + +if(CONFIG_VFS_SUPPORT_IO) + idf_component_optional_requires(PRIVATE vfs) +endif() + +if(CONFIG_SW_COEXIST_ENABLE OR CONFIG_EXTERNAL_COEX_ENABLE) + idf_component_optional_requires(PRIVATE esp_wifi) +endif() diff --git a/components/esp_system/startup.c b/components/esp_system/startup.c index 0bc753c368..786466b652 100644 --- a/components/esp_system/startup.c +++ b/components/esp_system/startup.c @@ -26,7 +26,6 @@ #include "esp_spi_flash.h" #include "esp_flash_internal.h" #include "esp_newlib.h" -#include "esp_vfs_dev.h" #include "esp_timer.h" #include "esp_efuse.h" #include "esp_flash_encrypt.h" @@ -41,7 +40,9 @@ /***********************************************/ // Headers for other components init functions +#if CONFIG_SW_COEXIST_ENABLE || CONFIG_EXTERNAL_COEX_ENABLE #include "esp_coexist_internal.h" +#endif #if CONFIG_ESP_COREDUMP_ENABLE #include "esp_core_dump.h" @@ -58,8 +59,12 @@ #include "esp_private/pm_impl.h" #endif -#include "esp_pthread.h" +#if CONFIG_VFS_SUPPORT_IO +#include "esp_vfs_dev.h" #include "esp_vfs_console.h" +#endif + +#include "esp_pthread.h" #include "esp_private/esp_clk.h" #include "esp_private/brownout.h" From ab6151fc8e8b756951b7c84bba2f98a7a6a160fd Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Sat, 16 Apr 2022 08:42:01 +0200 Subject: [PATCH 2/6] test_utils: add missing dependency on esp_netif --- tools/unit-test-app/components/test_utils/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/unit-test-app/components/test_utils/CMakeLists.txt b/tools/unit-test-app/components/test_utils/CMakeLists.txt index af19410c15..0cd755a176 100644 --- a/tools/unit-test-app/components/test_utils/CMakeLists.txt +++ b/tools/unit-test-app/components/test_utils/CMakeLists.txt @@ -26,4 +26,4 @@ idf_component_register(SRCS ${srcs} INCLUDE_DIRS include PRIV_INCLUDE_DIRS private_include REQUIRES spi_flash idf_test cmock - PRIV_REQUIRES perfmon driver) + PRIV_REQUIRES perfmon driver esp_netif) From 47659be5b83e9adc702e6b8198d344481be5d773 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Thu, 14 Apr 2022 20:03:56 +0200 Subject: [PATCH 3/6] build system: remove lwip from common requirements lwip was added to common requirements list to provide "sys/socket.h" header to all components without additional requirements specified. However, lwip pulls in a lot of dependencies on other components. This commit removes lwip from common requirements to reduce the number of components in G1-only apps. To compensate for this removal, the following changes are made: - newlib (which is a common requirement) has a public dependency on lwip if lwip is present in the build. This ensures that sys/socket.h is available as long as lwip component is included into the build. - lwip is now a public requirement of esp-tls since esp_tls.h includes sys/socket.h header. - lwip is now a public requirement o esp_http_client because sys/socket.h is included from esp_http_client.h - lwip is now a private requirement of esp_wifi for "smartconfig_ack" - lwip is now a private requirement of mqtt for socket functions - lwip is now a public requirement of tcp_transport because esp_transport_tcp.h includes sys/socket.h header. - mbedtls checks if lwip component is present in the build. If yes, net_sockets.c is added to the build, along with the dependency on lwip. Previously lwip was a public requirement of mbedtls unconditionally. system/g1_components test app is updated to reflect the changes Default public dependencies of a component before and after this change, except common requirements: - esp_timer (public dependency of freertos) - bootloader_support (public dependency of esp_hw_support) - vfs (public dependency of lwip) - esp_wifi (public dependency of lwip) - esp_event (public dependency of esp_wifi) - esp_netif (public dependency of esp_event) - esp_eth (public dependency of esp_netif) - esp_phy (public dependency of esp_wifi) After: - esp_timer (public dependency of freertos) - bootloader_support (public dependency of esp_hw_support) Altogether, the following components have been always added as public requirements to all other components, and are not added now ([breaking-change]): - lwip - vfs - esp_wifi - esp_event - esp_netif - esp_eth - esp_phy Application components now need to explicitly declare dependencies on these components. --- components/esp-tls/CMakeLists.txt | 6 ++-- components/esp_http_client/CMakeLists.txt | 3 +- components/esp_wifi/CMakeLists.txt | 2 +- components/mbedtls/CMakeLists.txt | 29 ++++++++++++------- components/mqtt/CMakeLists.txt | 1 + components/newlib/CMakeLists.txt | 4 +++ components/tcp_transport/CMakeLists.txt | 2 +- tools/cmake/build.cmake | 3 +- .../system/g1_components/CMakeLists.txt | 22 -------------- 9 files changed, 32 insertions(+), 40 deletions(-) diff --git a/components/esp-tls/CMakeLists.txt b/components/esp-tls/CMakeLists.txt index d53d4fe10e..1cdd1e5b69 100644 --- a/components/esp-tls/CMakeLists.txt +++ b/components/esp-tls/CMakeLists.txt @@ -12,8 +12,10 @@ endif() idf_component_register(SRCS "${srcs}" INCLUDE_DIRS . esp-tls-crypto PRIV_INCLUDE_DIRS "private_include" - REQUIRES mbedtls - PRIV_REQUIRES lwip http_parser) + # lwip and mbedtls are public requirements becasue esp_tls.h + # includes sys/socket.h and mbedtls header files. + REQUIRES mbedtls lwip + PRIV_REQUIRES http_parser) if(CONFIG_ESP_TLS_USING_WOLFSSL) idf_component_get_property(wolfssl esp-wolfssl COMPONENT_LIB) diff --git a/components/esp_http_client/CMakeLists.txt b/components/esp_http_client/CMakeLists.txt index 6794a62e2b..45e53d8413 100644 --- a/components/esp_http_client/CMakeLists.txt +++ b/components/esp_http_client/CMakeLists.txt @@ -4,5 +4,6 @@ idf_component_register(SRCS "esp_http_client.c" "lib/http_utils.c" INCLUDE_DIRS "include" PRIV_INCLUDE_DIRS "lib/include" - REQUIRES http_parser + # lwip is a public requirement because esp_http_client.h includes sys/socket.h + REQUIRES http_parser lwip PRIV_REQUIRES tcp_transport) diff --git a/components/esp_wifi/CMakeLists.txt b/components/esp_wifi/CMakeLists.txt index 4b37de238a..a3efef9a2f 100644 --- a/components/esp_wifi/CMakeLists.txt +++ b/components/esp_wifi/CMakeLists.txt @@ -31,7 +31,7 @@ idf_component_register(SRCS "${srcs}" INCLUDE_DIRS "include" REQUIRES esp_event esp_phy PRIV_REQUIRES driver esptool_py esp_netif esp_pm esp_timer nvs_flash - wpa_supplicant hal ${extra_priv_requires} + wpa_supplicant hal lwip ${extra_priv_requires} LDFRAGMENTS "${ldfragments}") if(CONFIG_ESP32_WIFI_ENABLED) diff --git a/components/mbedtls/CMakeLists.txt b/components/mbedtls/CMakeLists.txt index 36ad5e69f0..85741abf17 100644 --- a/components/mbedtls/CMakeLists.txt +++ b/components/mbedtls/CMakeLists.txt @@ -17,10 +17,17 @@ endif() idf_component_register(SRCS "${mbedtls_srcs}" INCLUDE_DIRS "${mbedtls_include_dirs}" - REQUIRES lwip PRIV_REQUIRES "${priv_requires}" ) +# Determine the type of mbedtls component library +if(mbedtls_srcs STREQUAL "") + # For no sources in component library we must use "INTERFACE" + set(linkage_type INTERFACE) +else() + set(linkage_type PUBLIC) +endif() + if(CONFIG_MBEDTLS_CERTIFICATE_BUNDLE) set(bundle_name "x509_crt_bundle") @@ -103,8 +110,7 @@ endif() set(mbedtls_targets mbedtls mbedcrypto mbedx509) -set(mbedtls_target_sources "${COMPONENT_DIR}/port/mbedtls_debug.c" - "${COMPONENT_DIR}/port/net_sockets.c") +set(mbedtls_target_sources "${COMPONENT_DIR}/port/mbedtls_debug.c") if(CONFIG_MBEDTLS_DYNAMIC_BUFFER) set(mbedtls_target_sources ${mbedtls_target_sources} @@ -114,6 +120,15 @@ set(mbedtls_target_sources ${mbedtls_target_sources} "${COMPONENT_DIR}/port/dynamic/esp_ssl_tls.c") endif() +# net_sockets.c should only be compiled if BSD socket functions are available. +# Do this by checking if lwip component is included into the build. +idf_build_get_property(build_components BUILD_COMPONENTS) +if(lwip IN_LIST build_components) + list(APPEND mbedtls_target_sources "${COMPONENT_DIR}/port/net_sockets.c") + idf_component_get_property(lwip_lib lwip COMPONENT_LIB) + target_link_libraries(${COMPONENT_LIB} ${linkage_type} ${lwip_lib}) +endif() + # Add port files to mbedtls targets target_sources(mbedtls PRIVATE ${mbedtls_target_sources}) @@ -240,14 +255,6 @@ set_property(TARGET mbedcrypto APPEND PROPERTY LINK_INTERFACE_LIBRARIES mbedtls) set_property(TARGET mbedcrypto APPEND PROPERTY LINK_LIBRARIES idf::driver idf::${target}) set_property(TARGET mbedcrypto APPEND PROPERTY INTERFACE_LINK_LIBRARIES idf::driver idf::${target}) -# Link mbedtls libraries to component library -if(mbedtls_srcs STREQUAL "") - # For no sources in component library we must use "INTERFACE" - set(linkage_type INTERFACE) -else() - set(linkage_type PUBLIC) -endif() - target_link_libraries(${COMPONENT_LIB} ${linkage_type} ${mbedtls_targets}) if(CONFIG_ESP_TLS_USE_DS_PERIPHERAL) diff --git a/components/mqtt/CMakeLists.txt b/components/mqtt/CMakeLists.txt index ac832e1d14..004389a47a 100644 --- a/components/mqtt/CMakeLists.txt +++ b/components/mqtt/CMakeLists.txt @@ -4,6 +4,7 @@ idf_component_register(SRCS "esp-mqtt/mqtt_client.c" "esp-mqtt/lib/platform_esp32_idf.c" INCLUDE_DIRS esp-mqtt/include PRIV_INCLUDE_DIRS "esp-mqtt/lib/include" + PRIV_REQUIRES lwip ) if(TEST_BUILD) diff --git a/components/newlib/CMakeLists.txt b/components/newlib/CMakeLists.txt index ab732217ba..5e3e4efa1e 100644 --- a/components/newlib/CMakeLists.txt +++ b/components/newlib/CMakeLists.txt @@ -53,3 +53,7 @@ if(CONFIG_NEWLIB_NANO_FORMAT) endif() add_subdirectory(port) + +# if lwip is included in the build, add it as a public requirement so that +# #include works without any special provisions. +idf_component_optional_requires(PUBLIC lwip) diff --git a/components/tcp_transport/CMakeLists.txt b/components/tcp_transport/CMakeLists.txt index 4b67ca7b21..7eb449365e 100644 --- a/components/tcp_transport/CMakeLists.txt +++ b/components/tcp_transport/CMakeLists.txt @@ -11,4 +11,4 @@ endif() idf_component_register(SRCS "${srcs}" INCLUDE_DIRS "include" PRIV_INCLUDE_DIRS "private_include" - REQUIRES esp-tls) + REQUIRES esp-tls lwip) diff --git a/tools/cmake/build.cmake b/tools/cmake/build.cmake index dfcd74d312..6c73fdf308 100644 --- a/tools/cmake/build.cmake +++ b/tools/cmake/build.cmake @@ -162,9 +162,8 @@ function(__build_init idf_path) else() # Set components required by all other components in the build # - # - lwip is here so that #include works without any special provisions # - esp_hw_support is here for backward compatibility - set(requires_common cxx newlib freertos esp_hw_support heap log lwip soc hal esp_rom esp_common esp_system) + set(requires_common cxx newlib freertos esp_hw_support heap log soc hal esp_rom esp_common esp_system) idf_build_set_property(__COMPONENT_REQUIRES_COMMON "${requires_common}") endif() diff --git a/tools/test_apps/system/g1_components/CMakeLists.txt b/tools/test_apps/system/g1_components/CMakeLists.txt index 03b9ba0e24..06d963b725 100644 --- a/tools/test_apps/system/g1_components/CMakeLists.txt +++ b/tools/test_apps/system/g1_components/CMakeLists.txt @@ -41,15 +41,6 @@ set(extra_components_which_shouldnt_be_included # Figure out if these components can exist without a dependency on efuse. # If not, see if esp_hw_support can provide minimal efuse component replacement in G1 build. efuse - # esp_eth is a dependency of esp_netif, esp_event, lwip - # Once they are removed from G1, esp_eth will be removed as well. - esp_eth - # esp_event is a dependency of esp_wifi, esp_eth. Both should be removed from G1. - esp_event - # esp_netif is a dependency of lwip and esp_event, should disappear once lwip is removed. - esp_netif - # esp_phy is a dependency of esp_wifi. - esp_phy # esp_pm is pulled in by freertos, can be made a weak dependency # conditional on related Kconfig option. It is also used by esp_wifi, driver, mbedtls, # all of which should be removed from G1-only build. @@ -59,30 +50,17 @@ set(extra_components_which_shouldnt_be_included # esp_timer is a dependency of freertos, esp_event, esp_wifi, driver. # For freertos, it can be made a weak dependency conditional on FREERTOS_RUN_TIME_STATS_USING_ESP_TIMER esp_timer - # esp_wifi is a dependency of lwip. - # [refactor-todo]: investigate making esp_wifi a conditional dependency. - esp_wifi # esptool_py is a dependency of bootloader, esp_wifi, app_update, partition_table, all of which # should be removed from G1-only build. esptool_py - # lwip is a common component due to "sys/socket.h" header. - # [refactor-todo] consider adding a system-level sys/socket.h in newlib instead - lwip # mbedtls is a dependency of bootloader_support (plus other easier-to-remove ones) # it is hard to make it conditional, need to remove bootloader_support. mbedtls - # nvs_flash is required by: - # esp_wifi, esp_phy — both should be removed - nvs_flash # partition_table is pulled in by app_update, esptool_py, bootloader; all to be removed partition_table # pthread is required by esp_system (for initialization only, can be made a weak dependency) # and cxx. See also [refactor-todo] about cxx, can it work without pthread? pthread - # vfs is a dependency of lwip. It can be made conditional, while lwip is still a common component. - vfs - # wpa_supplicant is a dependency of esp_wifi, which is to be removed from G1-only build - wpa_supplicant ) set(expected_components From 598e8db29cea5470987938a070cf4a4c3cd7d1cf Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Sat, 16 Apr 2022 08:08:37 +0200 Subject: [PATCH 4/6] docs: update build system reference and migration guides for lwip Previous commit removes lwip from the common requirements. This commit updates the documentation to describe the resulting changes. --- docs/en/api-guides/build-system.rst | 2 +- docs/en/migration-guides/build-system.rst | 7 +++++++ docs/zh_CN/api-guides/build-system.rst | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/docs/en/api-guides/build-system.rst b/docs/en/api-guides/build-system.rst index 89f4c6d230..9c2edd12e0 100644 --- a/docs/en/api-guides/build-system.rst +++ b/docs/en/api-guides/build-system.rst @@ -548,7 +548,7 @@ Common component requirements To avoid duplication, every component automatically requires some "common" IDF components even if they are not mentioned explicitly. Headers from these components can always be included. -The list of common components is: cxx, newlib, freertos, esp_hw_support, heap, log, lwip, soc, hal, esp_rom, esp_common, esp_system. +The list of common components is: cxx, newlib, freertos, esp_hw_support, heap, log, soc, hal, esp_rom, esp_common, esp_system. Including components in the build ---------------------------------- diff --git a/docs/en/migration-guides/build-system.rst b/docs/en/migration-guides/build-system.rst index b01fc66729..a21d121185 100644 --- a/docs/en/migration-guides/build-system.rst +++ b/docs/en/migration-guides/build-system.rst @@ -19,6 +19,13 @@ In previous versions of ESP-IDF, some components were always added as public req * ``driver`` * ``efuse`` * ``esp_timer`` +* ``lwip`` +* ``vfs`` +* ``esp_wifi`` +* ``esp_event`` +* ``esp_netif`` +* ``esp_eth`` +* ``esp_phy`` This means that it was possible to include header files of those components without specifying them as requirements in ``idf_component_register``. diff --git a/docs/zh_CN/api-guides/build-system.rst b/docs/zh_CN/api-guides/build-system.rst index d4ef1ecf52..e3f8266942 100644 --- a/docs/zh_CN/api-guides/build-system.rst +++ b/docs/zh_CN/api-guides/build-system.rst @@ -634,7 +634,7 @@ Spark Plug 组件 为避免重复性工作，各组件都用自动依赖一些“通用” IDF 组件，即使它们没有被明确提及。这些组件的头文件会一直包含在构建系统中。 -通用组件包括：cxx、newlib、freertos、esp_hw_support、heap、log、lwip、soc、hal、esp_rom、esp_common、esp_system。 +通用组件包括：cxx、newlib、freertos、esp_hw_support、heap、log、soc、hal、esp_rom、esp_common、esp_system。 在构建中导入组件 ----------------- From 77b754b47f1462145e995b774a0c0a8c4774a811 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 2 May 2022 20:43:40 +0200 Subject: [PATCH 5/6] newlib: fix return value of no-VFS _read_r if nothing received 'read' function should return 0 when encountering an end of file. When newlib calls read and sees EOF returned, it assumes that this condition is permanent and never calls 'read' for this file again (unless the read pointer is moved using fseek). The correct behavior in case no characters were received over UART is to return -1. In this case newlib will retry reading from file on next call to fread, fgetc or another function which calls __srefill_r. --- components/newlib/syscalls.c | 22 +++++++++------------- tools/ci/check_copyright_ignore.txt | 1 - 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/components/newlib/syscalls.c b/components/newlib/syscalls.c index e72fd14442..06e342407c 100644 --- a/components/newlib/syscalls.c +++ b/components/newlib/syscalls.c @@ -1,16 +1,8 @@ -// Copyright 2015-2020 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #include #include @@ -59,6 +51,10 @@ ssize_t _read_r_console(struct _reent *r, int fd, void * data, size_t size) break; } } + if (received == 0) { + errno = EWOULDBLOCK; + return -1; + } return received; } __errno_r(r) = EBADF; diff --git a/tools/ci/check_copyright_ignore.txt b/tools/ci/check_copyright_ignore.txt index d8a4ae1131..273b4f3088 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/tools/ci/check_copyright_ignore.txt @@ -1090,7 +1090,6 @@ components/newlib/poll.c components/newlib/priv_include/esp_time_impl.h components/newlib/pthread.c components/newlib/reent_init.c -components/newlib/syscalls.c components/newlib/termios.c components/newlib/test/test_atomic.c components/newlib/test/test_locks.c From 89e78976ab9344cfa6b403961c320aea1b8cdbd1 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 2 May 2022 20:46:41 +0200 Subject: [PATCH 6/6] tests: panic: make 'get_test_name' work without VFS, add echo When vfs component is not added to the build, bare minimum syscalls are provided by newlib component. These syscalls currently don't perform CR/LF translation. This commit makes the 'get_test_name' function work with minimal syscalls and also adds echo, so that the user sees what they type. --- tools/test_apps/system/panic/main/test_panic_main.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tools/test_apps/system/panic/main/test_panic_main.c b/tools/test_apps/system/panic/main/test_panic_main.c index f533a892f7..85e3d5bfbd 100644 --- a/tools/test_apps/system/panic/main/test_panic_main.c +++ b/tools/test_apps/system/panic/main/test_panic_main.c @@ -194,12 +194,17 @@ static const char* get_test_name(void) c = getchar(); if (c == EOF) { vTaskDelay(pdMS_TO_TICKS(10)); - } else if (c == '\r') { - continue; - } else if (c == '\n') { + } else if ((c == '\r' || c == '\n') && p != test_name_str) { + /* terminate the line */ + puts("\n\r"); + fflush(stdout); *p = '\0'; break; } else { + /* echo the received character */ + putchar(c); + fflush(stdout); + /* and save it */ *p = c; ++p; }