From 1ad3e2db17f93f64ae4f86200d59e4fa12c91b0c Mon Sep 17 00:00:00 2001 From: David Cermak Date: Fri, 13 May 2022 16:55:22 +0200 Subject: [PATCH 1/3] mqtt: Fix and add mqtt host test to CI --- .gitlab/ci/host-test.yml | 7 +++++++ components/mqtt/CMakeLists.txt | 21 ++++--------------- components/mqtt/host_test/CMakeLists.txt | 2 ++ .../mqtt/host_test/main/test_mqtt_client.cpp | 15 ++++++------- components/mqtt/host_test/mocks/config.yaml | 2 ++ .../mocks/include/local_FreeRTOS_config.h | 6 ++++++ 6 files changed, 27 insertions(+), 26 deletions(-) create mode 100644 components/mqtt/host_test/mocks/include/local_FreeRTOS_config.h diff --git a/.gitlab/ci/host-test.yml b/.gitlab/ci/host-test.yml index 29783e2201..a37e1cc763 100644 --- a/.gitlab/ci/host-test.yml +++ b/.gitlab/ci/host-test.yml @@ -358,6 +358,13 @@ test_nvs_page: - idf.py build - build/test_nvs_page_host.elf +test_mqtt_on_host: + extends: .host_test_template + script: + - cd ${IDF_PATH}/components/mqtt/host_test + - idf.py build + - LSAN_OPTIONS=verbosity=1:log_threads=1 build/host_mqtt_client_test.elf + test_log: extends: .host_test_template script: diff --git a/components/mqtt/CMakeLists.txt b/components/mqtt/CMakeLists.txt index 004389a47a..ae35aefc02 100644 --- a/components/mqtt/CMakeLists.txt +++ b/components/mqtt/CMakeLists.txt @@ -3,9 +3,7 @@ idf_component_register(SRCS "esp-mqtt/mqtt_client.c" "esp-mqtt/lib/mqtt_outbox.c" "esp-mqtt/lib/platform_esp32_idf.c" INCLUDE_DIRS esp-mqtt/include - PRIV_INCLUDE_DIRS "esp-mqtt/lib/include" - PRIV_REQUIRES lwip - ) + PRIV_INCLUDE_DIRS "esp-mqtt/lib/include") if(TEST_BUILD) message(STATUS "building MOCKS") @@ -13,10 +11,9 @@ idf_component_get_property(tcp_transport_dir tcp_transport COMPONENT_DIR) idf_component_get_property(esp_hw_support_dir esp_hw_support COMPONENT_DIR) idf_component_get_property(esp_event_dir esp_event COMPONENT_DIR) idf_component_get_property(log_dir log COMPONENT_DIR) -idf_component_get_property(freertos_dir freertos COMPONENT_DIR) +idf_component_get_property(freertos_dir freertos COMPONENT_OVERRIDEN_DIR) idf_component_get_property(http_parser_dir http_parser COMPONENT_DIR) idf_component_get_property(esp_wifi_dir esp_wifi COMPONENT_DIR) -idf_component_get_property(esp_hw_support_dir esp_hw_support COMPONENT_DIR) idf_component_get_property(esp_tls_dir esp-tls COMPONENT_DIR) idf_component_get_property(esp_netif_dir esp_netif COMPONENT_DIR) idf_component_get_property(esp_common_dir esp_common COMPONENT_DIR) @@ -56,10 +53,6 @@ idf_component_get_property(mbedtls_dir mbedtls COMPONENT_DIR) ${esp_event_dir}/include/esp_event.h ${esp_hw_support_dir}/include/esp_mac.h ${esp_hw_support_dir}/include/esp_random.h - ${esp_system_dir}/include/esp_system.h - ${esp_tls_dir}/esp_tls.h - ${freertos_dir}/FreeRTOS-Kernel/include/freertos/queue.h - ${freertos_dir}/FreeRTOS-Kernel/include/freertos/task.h ${freertos_dir}/FreeRTOS-Kernel/include/freertos/event_groups.h ${log_dir}/include/esp_log.h ${http_parser_dir}/http_parser.h @@ -74,13 +67,9 @@ idf_component_get_property(mbedtls_dir mbedtls COMPONENT_DIR) ${MOCK_GEN_DIR}/Mockesp_event.c ${MOCK_GEN_DIR}/Mockesp_mac.c ${MOCK_GEN_DIR}/Mockesp_random.c - ${MOCK_GEN_DIR}/Mockesp_system.c - ${MOCK_GEN_DIR}/Mockesp_tls.c ${MOCK_GEN_DIR}/Mockesp_log.c ${MOCK_GEN_DIR}/Mockhttp_parser.c ${MOCK_GEN_DIR}/Mockevent_groups.c - ${MOCK_GEN_DIR}/Mockqueue.c - ${MOCK_GEN_DIR}/Mocktask.c ) add_custom_command( @@ -122,10 +111,6 @@ idf_component_get_property(mbedtls_dir mbedtls COMPONENT_DIR) ) target_link_libraries(mocks PUBLIC ${cmock_lib}) target_compile_definitions(mocks PUBLIC - CONFIG_LOG_MAXIMUM_LEVEL=5 - CONFIG_LOG_DEFAULT_LEVEL=3 - CONFIG_ESP_TLS_USING_MBEDTLS - CONFIG_ESP_TLS_SERVER CONFIG_LOG_TIMESTAMP_SOURCE_RTOS) target_link_options(${COMPONENT_LIB} INTERFACE -fsanitize=address) @@ -134,5 +119,7 @@ idf_component_get_property(mbedtls_dir mbedtls COMPONENT_DIR) else() idf_component_get_property(http_parser_lib http_parser COMPONENT_LIB) idf_component_get_property(tcp_transport_lib tcp_transport COMPONENT_LIB) + idf_component_get_property(lwip_lib lwip COMPONENT_LIB) target_link_libraries(${COMPONENT_LIB} PUBLIC ${http_parser_lib} ${tcp_transport_lib}) + target_link_libraries(${COMPONENT_LIB} PRIVATE ${lwip_lib}) endif() diff --git a/components/mqtt/host_test/CMakeLists.txt b/components/mqtt/host_test/CMakeLists.txt index 947809f26d..fedde21076 100644 --- a/components/mqtt/host_test/CMakeLists.txt +++ b/components/mqtt/host_test/CMakeLists.txt @@ -2,5 +2,7 @@ cmake_minimum_required(VERSION 3.16) include($ENV{IDF_PATH}/tools/cmake/project.cmake) set(COMPONENTS main) +list(APPEND EXTRA_COMPONENT_DIRS "$ENV{IDF_PATH}/tools/mocks/freertos/") + option(TEST_BUILD "" ON) project(host_mqtt_client_test) diff --git a/components/mqtt/host_test/main/test_mqtt_client.cpp b/components/mqtt/host_test/main/test_mqtt_client.cpp index 51ef9cf8bc..58dd134a96 100644 --- a/components/mqtt/host_test/main/test_mqtt_client.cpp +++ b/components/mqtt/host_test/main/test_mqtt_client.cpp @@ -1,11 +1,9 @@ #define CATCH_CONFIG_MAIN // This tells the catch header to generate a main #include "catch.hpp" -#include "mqtt_client.h" extern "C" { #include "Mockesp_event.h" #include "Mockesp_log.h" -#include "Mockesp_system.h" #include "Mockesp_mac.h" #include "Mockesp_transport.h" #include "Mockesp_transport_ssl.h" @@ -20,17 +18,14 @@ extern "C" { * The following functions are not directly called but the generation of them * from cmock is broken, so we need to define them here. */ - BaseType_t xQueueTakeMutexRecursive(QueueHandle_t xMutex, - TickType_t xTicksToWait) + esp_err_t esp_tls_get_and_clear_last_error(esp_tls_error_handle_t h, int *esp_tls_code, int *esp_tls_flags) { - return 0; - } - BaseType_t xQueueGiveMutexRecursive(QueueHandle_t xMutex) - { - return 0; + return ESP_OK; } } +#include "mqtt_client.h" + struct ClientInitializedFixture { esp_mqtt_client_handle_t client; ClientInitializedFixture() @@ -42,6 +37,8 @@ struct ClientInitializedFixture { int event_group; uint8_t mac[] = {0xAA, 0x55, 0xAA, 0x55, 0xAA, 0x55}; esp_log_write_Ignore(); + xQueueTakeMutexRecursive_CMockIgnoreAndReturn(0, true); + xQueueGiveMutexRecursive_CMockIgnoreAndReturn(0, true); xQueueCreateMutex_ExpectAnyArgsAndReturn( reinterpret_cast(&mtx)); xEventGroupCreate_IgnoreAndReturn(reinterpret_cast(&event_group)); diff --git a/components/mqtt/host_test/mocks/config.yaml b/components/mqtt/host_test/mocks/config.yaml index 8fc80e9a55..cdaab2ae2b 100644 --- a/components/mqtt/host_test/mocks/config.yaml +++ b/components/mqtt/host_test/mocks/config.yaml @@ -7,6 +7,8 @@ - array - callback :includes_h_pre_orig_header: + - local_FreeRTOS_config.h + - esp_attr.h - FreeRTOS.h - net/if.h :strippables: diff --git a/components/mqtt/host_test/mocks/include/local_FreeRTOS_config.h b/components/mqtt/host_test/mocks/include/local_FreeRTOS_config.h new file mode 100644 index 0000000000..e7e1014beb --- /dev/null +++ b/components/mqtt/host_test/mocks/include/local_FreeRTOS_config.h @@ -0,0 +1,6 @@ +/* + * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ +#define configUSE_TRACE_FACILITY 1 From 0aea4bf50d971b00a9c732391a635f4fadb4509d Mon Sep 17 00:00:00 2001 From: David Cermak Date: Wed, 8 Jun 2022 20:07:04 +0200 Subject: [PATCH 2/3] mqtt: Fix client_enqueue(len=0), Improve transport memory * Update submodule: git log --oneline 64f88b4412ea6649dbf207a07370c2617160d044..a21c387d6280260894981c22494017c893d505b9 Detailed description of the changes: * mqtt_client: Added checks for cleanly-closed connection and timeout - See merge request espressif/esp-mqtt!118 - Added checks for cleanly-closed connection and timeout (espressif/esp-mqtt@e05d873) * mqtt_client: fix esp_mqtt_client_enqueue for len=0 (GitHub PR) - See merge request espressif/esp-mqtt!135 - mqtt_client: fix esp_mqtt_client_enqueue for len=0 (espressif/esp-mqtt@69b6493) * Fix implicit malloc/free inclusion - See merge request espressif/esp-mqtt!134 - See commit https://github.com/espressif/esp-mqtt/commit/9299f54 * feat(mqtt): Optimize mqtt transport list and remove unused transport - See merge request espressif/esp-mqtt!131 - See commit https://github.com/espressif/esp-mqtt/commit/647e0ef * Fix WSS default port selection through menuconfig. - See merge request espressif/esp-mqtt!132 - - Closes https://github.com/espressif/esp-mqtt/issues/223 - See commit https://github.com/espressif/esp-mqtt/commit/f6caaff --- components/mqtt/esp-mqtt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/mqtt/esp-mqtt b/components/mqtt/esp-mqtt index 64f88b4412..a21c387d62 160000 --- a/components/mqtt/esp-mqtt +++ b/components/mqtt/esp-mqtt @@ -1 +1 @@ -Subproject commit 64f88b4412ea6649dbf207a07370c2617160d044 +Subproject commit a21c387d6280260894981c22494017c893d505b9 From 877eb626021f07e27066a8745ba5f5f41cf8b41e Mon Sep 17 00:00:00 2001 From: David Cermak Date: Thu, 9 Jun 2022 11:08:45 +0200 Subject: [PATCH 3/3] mqtt: Update tests to start with valid transport --- .../mqtt/host_test/main/test_mqtt_client.cpp | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/components/mqtt/host_test/main/test_mqtt_client.cpp b/components/mqtt/host_test/main/test_mqtt_client.cpp index 58dd134a96..a28cbc2092 100644 --- a/components/mqtt/host_test/main/test_mqtt_client.cpp +++ b/components/mqtt/host_test/main/test_mqtt_client.cpp @@ -69,7 +69,11 @@ struct ClientInitializedFixture { }; TEST_CASE_METHOD(ClientInitializedFixture, "Client set uri") { - struct http_parser_url ret_uri; + struct http_parser_url ret_uri = { + .field_set = 1, + .port = 0, + .field_data = { { 0, 1} } + }; SECTION("User set a correct URI") { http_parser_parse_url_StopIgnore(); http_parser_parse_url_ExpectAnyArgsAndReturn(0); @@ -88,8 +92,20 @@ TEST_CASE_METHOD(ClientInitializedFixture, "Client set uri") TEST_CASE_METHOD(ClientInitializedFixture, "Client Start") { SECTION("Successful start") { + esp_mqtt_client_config_t config{}; + config.uri = "mqtt://1.1.1.1"; + struct http_parser_url ret_uri = { + .field_set = 1 | (1<<1), + .port = 0, + .field_data = { { 0, 4 } /*mqtt*/, { 7, 1 } } // at least *scheme* and *host* + }; + http_parser_parse_url_StopIgnore(); + http_parser_parse_url_ExpectAnyArgsAndReturn(0); + http_parser_parse_url_ReturnThruPtr_u(&ret_uri); xTaskCreatePinnedToCore_ExpectAnyArgsAndReturn(pdTRUE); - auto res = esp_mqtt_client_start(client); + auto res = esp_mqtt_set_config(client, &config); + REQUIRE(res == ESP_OK); + res = esp_mqtt_client_start(client); REQUIRE(res == ESP_OK); } SECTION("Failed on initialization") {