Fix warnings

.gitmodules

@@ -1,6 +1,3 @@
-[submodule "gtest"]
-	path = src/third_party/gtest
-	url = https://github.com/google/googletest
 [submodule "mavsdk-proto"]
 	path = proto
 	url = https://github.com/mavlink/MAVSDK-Proto.git

CMakeLists.txt

@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.13)
+cmake_minimum_required(VERSION 3.14)
 
 option(HUNTER_ENABLED "Enable Hunter package manager support" OFF)
 option(SUPERBUILD "Build dependencies" ON)

src/CMakeLists.txt

@@ -37,10 +37,9 @@ endif()
 
 # This will noop if hunter is not enabled
 hunter_add_package(jsoncpp)
-hunter_add_package(CURL)
 
+hunter_add_package(CURL)
 find_package(CURL REQUIRED)
-find_package(LibLZMA REQUIRED)
 
 if(BUILD_TESTS AND (IOS OR ANDROID))
     message(STATUS "Building for iOS or Android: forcing BUILD_TESTS to FALSE...")
@@ -71,12 +70,41 @@ include(cmake/static_analyzers.cmake)
 
 if(BUILD_TESTS)
     enable_testing()
-    add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/third_party/gtest EXCLUDE_FROM_ALL)
 
-    add_subdirectory(integration_tests)
+    hunter_add_package(GTest)
+    find_package(GTest REQUIRED)
 
-    add_subdirectory(unit_tests)
+    # Check if the GTest::gtest target exists, for older cmake with Ubuntu 20.04
+
+    # Check and create GTest::gtest if not available
+    if(NOT TARGET GTest::gtest)
+        add_library(GTest::gtest INTERFACE IMPORTED)
+        set_property(TARGET GTest::gtest PROPERTY
+            INTERFACE_INCLUDE_DIRECTORIES ${GTEST_INCLUDE_DIRS})
+        set_property(TARGET GTest::gtest PROPERTY
+            INTERFACE_LINK_LIBRARIES ${GTEST_BOTH_LIBRARIES})
+    endif()
 
+    # Check and create GTest::gtest_main if not available
+    if(NOT TARGET GTest::gtest_main)
+        add_library(GTest::gtest_main INTERFACE IMPORTED)
+        set_property(TARGET GTest::gtest_main PROPERTY
+            INTERFACE_INCLUDE_DIRECTORIES ${GTEST_INCLUDE_DIRS})
+        set_property(TARGET GTest::gtest_main PROPERTY
+            INTERFACE_LINK_LIBRARIES ${GTEST_MAIN_LIBRARIES})
+    endif()
+
+    # Check and create GTest::gmock if not available
+    if(NOT TARGET GTest::gmock)
+        add_library(GTest::gmock INTERFACE IMPORTED)
+        set_property(TARGET GTest::gmock PROPERTY
+            INTERFACE_INCLUDE_DIRECTORIES ${GMOCK_INCLUDE_DIRS})
+        set_property(TARGET GTest::gmock PROPERTY
+            INTERFACE_LINK_LIBRARIES ${GMOCK_BOTH_LIBRARIES})
+    endif()
+
+    add_subdirectory(integration_tests)
+    add_subdirectory(unit_tests)
     add_subdirectory(system_tests)
 endif()
 

src/cmake/compiler_flags.cmake

@@ -27,9 +27,6 @@ else()
     else()
         add_definitions(-fno-exceptions)
         set(warnings "-Wall -Wextra -Wshadow -Wno-strict-aliasing -Wold-style-cast -Wdouble-promotion -Wformat=2 -Wno-address-of-packed-member")
-        if (WERROR)
-            set(warnings "${warnings} -Werror")
-        endif()
     endif()
 
 
@@ -41,7 +38,7 @@ else()
             set(warnings "${warnings} -Wduplicated-cond -Wnull-dereference")
         endif()
         if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 7)
-            set(warnings "${warnings} -Wduplicated-branches")
+            set(warnings "${warnings} -Wduplicated-branches -Wno-psabi")
         endif()
 
         if(NOT CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 5)
@@ -61,6 +58,8 @@ else()
 
     # Otherwise tinyxml2 complains.
     set(warnings "${warnings} -Wno-old-style-cast")
+
+    set(warnings "${warnings} -Wno-double-promotion -Wno-sign-compare")
 endif()
 
 if(APPLE)
@@ -79,19 +78,26 @@ if(UNIX AND NOT APPLE)
     add_definitions("-DLINUX")
 endif()
 
+if(WERROR)
+    set(warnings "${warnings} -Werror")
+endif()
+
+set(CMAKE_C_FLAGS "${warnings} ${CMAKE_C_FLAGS}")
+set(CMAKE_CXX_FLAGS "${warnings} ${CMAKE_CXX_FLAGS}")
+
 if(ASAN)
     set(CMAKE_C_FLAGS "-fsanitize=address ${CMAKE_C_FLAGS}")
-    set(CMAKE_CXX_FLAGS "-fsanitize=address ${CMAKE_C_FLAGS}")
+    set(CMAKE_CXX_FLAGS "-fsanitize=address ${CMAKE_CXX_FLAGS}")
 endif()
 
 if(UBSAN)
     set(CMAKE_C_FLAGS "-fsanitize=undefined ${CMAKE_C_FLAGS}")
-    set(CMAKE_CXX_FLAGS "-fsanitize=undefined ${CMAKE_C_FLAGS}")
+    set(CMAKE_CXX_FLAGS "-fsanitize=undefined ${CMAKE_CXX_FLAGS}")
 endif()
 
 if(LSAN)
     set(CMAKE_C_FLAGS "-fsanitize=leak ${CMAKE_C_FLAGS}")
-    set(CMAKE_CXX_FLAGS "-fsanitize=leak ${CMAKE_C_FLAGS}")
+    set(CMAKE_CXX_FLAGS "-fsanitize=leak ${CMAKE_CXX_FLAGS}")
 endif()
 
 if(TSAN)

src/integration_tests/CMakeLists.txt

@@ -59,26 +59,6 @@ target_include_directories(integration_tests_runner
     ${PROJECT_SOURCE_DIR}/mavsdk/core
 )
 
-# SYSTEM because we don't want warnings for gtest headers.
-target_include_directories(integration_tests_runner
-    SYSTEM
-    PRIVATE ${PROJECT_SOURCE_DIR}/third_party/gtest/googletest/include
-    PRIVATE ${PROJECT_SOURCE_DIR}/third_party/gtest/googlemock/include
-    PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/../third_party/include
-)
-
-if(MSVC)
-    target_compile_options(integration_tests_runner
-        PRIVATE
-         /wd4996 # ignore deprecated
-    )
-else()
-    target_compile_options(integration_tests_runner
-        PRIVATE
-        -Wno-error=deprecated-declarations
-    )
-endif()
-
 target_compile_definitions(integration_tests_runner
     PRIVATE
     "-DTESTING")
@@ -90,9 +70,9 @@ set_target_properties(integration_tests_runner
 target_link_libraries(integration_tests_runner
     PRIVATE
     mavsdk
-    gtest
-    gtest_main
-    gmock
+    GTest::gtest
+    GTest::gtest_main
+    GTest::gmock
 )
 
 add_test(integration_tests

src/mavsdk/core/CMakeLists.txt

@@ -3,6 +3,9 @@ find_package(Threads REQUIRED)
 
 find_package(MAVLink REQUIRED)
 
+hunter_add_package(lzma)
+find_package(LibLZMA REQUIRED)
+
 get_target_property(
     MAVLINK_INCLUDE_DIR
     MAVLink::mavlink
@@ -65,11 +68,7 @@ cmake_policy(SET CMP0079 NEW)
 target_link_libraries(mavsdk
     PRIVATE
     Threads::Threads
-    ${LIBLZMA_LIBRARIES}
-)
-target_include_directories(mavsdk
-    PRIVATE
-    ${LIBLZMA_INCLUDE_DIRS}
+    LibLZMA::LibLZMA
 )
 
 if (NOT BUILD_WITHOUT_CURL)

src/mavsdk/core/curl_wrapper.cpp

@@ -45,32 +45,6 @@ bool CurlWrapper::download_text(const std::string& url, std::string& content)
     }
 }
 
-static int
-upload_progress_update(void* p, double dltotal, double dlnow, double ultotal, double ulnow)
-{
-    UNUSED(dltotal);
-    UNUSED(dlnow);
-
-    auto* myp = reinterpret_cast<struct UpProgress*>(p);
-
-    if (myp->progress_callback == nullptr) {
-        return 0;
-    }
-
-    if (ultotal == 0 || ulnow == 0) {
-        return myp->progress_callback(0, Status::Idle, CURLcode::CURLE_OK);
-    }
-
-    int percentage = static_cast<int>(100.0 / ultotal * ulnow);
-
-    if (percentage > myp->progress_in_percentage) {
-        myp->progress_in_percentage = percentage;
-        return myp->progress_callback(percentage, Status::Uploading, CURLcode::CURLE_OK);
-    }
-
-    return 0;
-}
-
 size_t get_file_size(const std::string& path)
 {
     std::streampos begin, end;
@@ -89,68 +63,8 @@ template<typename T> std::string to_string(T value)
     return os.str();
 }
 
-bool CurlWrapper::upload_file(
-    const std::string& url, const std::string& path, const ProgressCallback& progress_callback)
-{
-    auto curl = std::shared_ptr<CURL>(curl_easy_init(), curl_easy_cleanup);
-    CURLcode res;
-
-    if (nullptr != curl) {
-        struct UpProgress progress;
-        progress.progress_callback = progress_callback;
-
-        curl_httppost* post = nullptr;
-        curl_httppost* last = nullptr;
-
-        struct curl_slist* chunk = nullptr;
-
-        // avoid sending 'Expect: 100-Continue' header, required by some server implementations
-        chunk = curl_slist_append(chunk, "Expect:");
-
-        // disable chunked upload
-        chunk = curl_slist_append(chunk, "Content-Encoding: ");
-
-        // to allow efficient file upload, we need to add the file size to the header
-        std::string filesize_header = "File-Size: " + to_string(get_file_size(path));
-        chunk = curl_slist_append(chunk, filesize_header.c_str());
-
-        curl_formadd(
-            &post, &last, CURLFORM_COPYNAME, "file", CURLFORM_FILE, path.c_str(), CURLFORM_END);
-
-        curl_easy_setopt(curl.get(), CURLOPT_CONNECTTIMEOUT, 5L);
-        curl_easy_setopt(curl.get(), CURLOPT_PROGRESSFUNCTION, upload_progress_update);
-        curl_easy_setopt(curl.get(), CURLOPT_PROGRESSDATA, &progress);
-        curl_easy_setopt(curl.get(), CURLOPT_VERBOSE, 1L);
-        curl_easy_setopt(curl.get(), CURLOPT_HTTPHEADER, chunk);
-        curl_easy_setopt(curl.get(), CURLOPT_URL, url.c_str());
-        curl_easy_setopt(curl.get(), CURLOPT_HTTPPOST, post);
-        curl_easy_setopt(curl.get(), CURLOPT_NOPROGRESS, 0L);
-
-        res = curl_easy_perform(curl.get());
-
-        curl_slist_free_all(chunk);
-        curl_formfree(post);
-
-        if (res == CURLcode::CURLE_OK) {
-            if (nullptr != progress_callback) {
-                progress_callback(100, Status::Finished, CURLcode::CURLE_OK);
-            }
-            return true;
-        } else {
-            if (nullptr != progress_callback) {
-                progress_callback(0, Status::Error, res);
-            }
-            LogErr() << "Error while uploading file, curl error code: " << curl_easy_strerror(res);
-            return false;
-        }
-    } else {
-        LogErr() << "Error: cannot start uploading because of curl initialization error.";
-        return false;
-    }
-}
-
-static int
-download_progress_update(void* p, double dltotal, double dlnow, double ultotal, double ulnow)
+static int download_progress_update(
+    void* p, curl_off_t dltotal, curl_off_t dlnow, curl_off_t ultotal, curl_off_t ulnow)
 {
     UNUSED(ultotal);
     UNUSED(ulnow);
@@ -188,7 +102,7 @@ bool CurlWrapper::download_file_to_path(
 
         fp = fopen(path.c_str(), "wb");
         curl_easy_setopt(curl.get(), CURLOPT_CONNECTTIMEOUT, 5L);
-        curl_easy_setopt(curl.get(), CURLOPT_PROGRESSFUNCTION, download_progress_update);
+        curl_easy_setopt(curl.get(), CURLOPT_XFERINFOFUNCTION, download_progress_update);
         curl_easy_setopt(curl.get(), CURLOPT_PROGRESSDATA, &progress);
         curl_easy_setopt(curl.get(), CURLOPT_URL, url.c_str());
         curl_easy_setopt(curl.get(), CURLOPT_WRITEFUNCTION, NULL);

src/mavsdk/core/curl_wrapper.h

@@ -21,10 +21,6 @@ class ICurlWrapper {
         const std::string& url,
         const std::string& path,
         const ProgressCallback& progress_callback) = 0;
-    virtual bool upload_file(
-        const std::string& url,
-        const std::string& path,
-        const ProgressCallback& progress_callback) = 0;
 };
 
 class CurlWrapper : public ICurlWrapper {
@@ -37,10 +33,6 @@ class CurlWrapper : public ICurlWrapper {
         const std::string& url,
         const std::string& path,
         const ProgressCallback& progress_callback) override;
-    bool upload_file(
-        const std::string& url,
-        const std::string& path,
-        const ProgressCallback& progress_callback) override;
 };
 
 #ifdef TESTING
@@ -53,12 +45,6 @@ class CurlWrapperMock : public ICurlWrapper {
             const std::string& url,
             const std::string& path,
             const ProgressCallback& progress_callback));
-    MOCK_METHOD3(
-        upload_file,
-        bool(
-            const std::string& url,
-            const std::string& path,
-            const ProgressCallback& progress_callback));
 };
 #endif // TESTING
 

src/mavsdk/core/http_loader.cpp

@@ -54,22 +54,6 @@ void HttpLoader::download_async(
     _work_queue.enqueue(work_item);
 }
 
-bool HttpLoader::upload_sync(const std::string& target_url, const std::string& local_path)
-{
-    auto work_item = std::make_shared<UploadItem>(target_url, local_path, nullptr);
-    bool success = do_upload(work_item, _curl_wrapper);
-    return success;
-}
-
-void HttpLoader::upload_async(
-    const std::string& target_url,
-    const std::string& local_path,
-    const ProgressCallback& progress_callback)
-{
-    auto work_item = std::make_shared<UploadItem>(target_url, local_path, progress_callback);
-    _work_queue.enqueue(work_item);
-}
-
 void HttpLoader::work_thread(HttpLoader* self)
 {
     while (!self->_should_exit) {
@@ -90,12 +74,6 @@ void HttpLoader::do_item(
         do_download(download_item, curl_wrapper);
         return;
     }
-
-    auto upload_item = std::dynamic_pointer_cast<UploadItem>(item);
-    if (nullptr != upload_item) {
-        do_upload(upload_item, curl_wrapper);
-        return;
-    }
 }
 
 bool HttpLoader::do_download(
@@ -106,14 +84,6 @@ bool HttpLoader::do_download(
     return success;
 }
 
-bool HttpLoader::do_upload(
-    const std::shared_ptr<UploadItem>& item, const std::shared_ptr<ICurlWrapper>& curl_wrapper)
-{
-    bool success = curl_wrapper->upload_file(
-        item->get_target_url(), item->get_local_path(), item->get_progress_callback());
-    return success;
-}
-
 bool HttpLoader::download_text_sync(const std::string& url, std::string& content)
 {
     bool success = _curl_wrapper->download_text(url, content);