Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New recipe: MAVSDK #25949

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions recipes/mavsdk/all/conandata.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
sources:
"0.39.0":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this version specifically? It's over 3 years old with version 2 already out

sdk:
url: "https://github.com/mavlink/MAVSDK/archive/refs/tags/v0.39.0.tar.gz"
sha256: "b7e4ff1fcce37ed88990d1bb27a754ede9575f52dcdf4d92b8f3bc7b4d8006d0"
mavlink:
url: "https://github.com/mavlink/c_library_v2.git"
commit: "87d55c503a52f8623d9fee34c1b6a2585bc0a236"
167 changes: 167 additions & 0 deletions recipes/mavsdk/all/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
from os.path import join
from conan import ConanFile
from conan.tools.files import get, copy
from conan.tools.files import replace_in_file, rmdir
from conan.tools.cmake import CMake, CMakeToolchain, CMakeDeps, cmake_layout
from conan.tools.scm import Git
from conan.tools.env import Environment

required_conan_version = ">=1.53.0"


class MAVSDKConan(ConanFile):
name = "mavsdk"
license = "BSD-3-Clause"
author = "SINTEF Ocean"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
author = "SINTEF Ocean"

authors in CCI recipes don't have a clear meaning, so we don't add them

homepage = "https://mavsdk.mavlink.io/main/en/"
url = "https://github.com/mavlink/MAVSDK.git"
description = "C++ library to interface with MAVLink systems"
settings = "os", "compiler", "build_type", "arch"
package_type = "library"
options = {
"fPIC": [True, False],
"shared": [True, False]
}
default_options = {
"fPIC": True,
"shared": False
}

@property
def export_sources(self):
pass
#export_conandata_patches(self)

def config_options(self):
if self.settings.os == "Windows":
del self.options.fPIC
Comment on lines +35 to +37
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def config_options(self):
if self.settings.os == "Windows":
del self.options.fPIC
implements = ["auto_shared_fpic"]

will take care of this, and the missing configure() method that handles fpic values when building shared libraries


def layout(self):
cmake_layout(self, src_folder="src")

def requirements(self):
self.requires("jsoncpp/[>=1.9.5 <2]")
self.requires("tinyxml2/[>=9.0.0 <10]")
self.requires("libcurl/[>=7.86.0 <8]")
Comment on lines +43 to +45
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now version ranges are limited to a select set of libraries, check https://github.com/conan-io/conan-center-index/blob/master/docs/adding_packages/dependencies.md#version-ranges, which means that of these 3, only libcurl can use version ranges. Sorry for the confussion :)

Also, if curl's 7.86 is the minimunm required, then great! But the lower the range, the more likely this won't cause issues to other elements on the graph (We have [>=7.78 <9] documented as the desired range by default, for example)


def configure(self):
if self.options.shared:
self.options.rm_safe("fPIC")

def build_requirements(self):
pass

def source(self):
get(self, **self.conan_data["sources"][self.version]["sdk"], strip_root=True)
sources = self.conan_data["sources"][self.version]["mavlink"]
mavlink_dir = join(self.source_folder, "src", "third_party",
"mavlink", "include", "mavlink", "v2.0")
git = Git(self, folder=mavlink_dir)
git.fetch_commit(sources["url"], sources["commit"])
Comment on lines +55 to +60
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
get(self, **self.conan_data["sources"][self.version]["sdk"], strip_root=True)
sources = self.conan_data["sources"][self.version]["mavlink"]
mavlink_dir = join(self.source_folder, "src", "third_party",
"mavlink", "include", "mavlink", "v2.0")
git = Git(self, folder=mavlink_dir)
git.fetch_commit(sources["url"], sources["commit"])
get(self, **self.conan_data["sources"][self.version]["sdk"], strip_root=True)
sources = self.conan_data["sources"][self.version]["mavlink"]
mavlink_dir = join(self.source_folder, "src", "third_party",
"mavlink", "include", "mavlink", "v2.0")
git = Git(self, folder=mavlink_dir)
git.fetch_commit(sources["url"], sources["commit"])

I'd like to understand a bit better the relationship between the sdk and the c library. When building the SDK, why do we need to provide the library separatedly? Is it a dependency? If so, maybe the best approach would be to split this into two - the c library, and the SDK that depends on it

Also, using git repositories in CCI is not a good idea, we should move the library to download a file just like the SDK does. If the library does not provide tags, we can still create versions from the commits themselves if needed :)


# TODO: create instead patch files to simplify building versions
replace_in_file(self, join(self.source_folder, "CMakeLists.txt"),
'message(STATUS "Version: ${VERSION_STR}")',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? How does this help the build process?

Either way, this might be better suited to be added as part of the CMakeToolchain variables if the variable is needed, something like this in the generate method if this is 100% needed:

tc = CMakeToolchain(self)
tc.variables["VERSION_STR"] = f"v{self.version}"

f'set(VERSION_STR v{self.version})\nmessage(STATUS "Version: ${{VERSION_STR}}")')

replace_in_file(self, join(self.source_folder, "src/core/connection.h"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a fix for an issue known upstream? Has it been reported? Having a link for traceability would be great!

"#include <unordered_set>",
"#include <unordered_set>\n#include <atomic>")

link_jsonpp = ["src/cmake/unit_tests.cmake",
"src/plugins/mission_raw/CMakeLists.txt",
"src/plugins/mission/CMakeLists.txt"]

for f in link_jsonpp:
replace_in_file(self, join(self.source_folder, f),
"JsonCpp::jsoncpp", "JsonCpp::JsonCpp")
Comment on lines +76 to +77
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be better fixed by setting this in the generate method:

deps = CMakeDeps(self)
deps.set_property("jsoncppp", "cmake_target_name", "JsonCpp::jsoncpp")
deps.generate()

This way Conan will change the generated target name to match those used by the project, removing the need to patch the sources themselves!


def generate(self):
tc = CMakeToolchain(self)
tc.variables["SUPERBUILD"] = False
tc.variables["BUILD_MAVSDK_SERVER"] = False
tc.generate()

deps = CMakeDeps(self)
deps.generate()


def build(self):
#apply_conandata_patches(self)
cmake = CMake(self)
cmake.configure()
cmake.build()

def package(self):
cmake = CMake(self)
cmake.install()

copy(self, "LICENSE", self.source_folder,
join(self.package_folder, "licenses"))
rmdir(self, join(self.package_folder, "lib", "cmake"))
if self.options.shared:
copy(self, "*mavsdk*.dll", dst=join(self.package_folder, "bin"), src=join(self.build_folder, "src"), keep_path=False)
Comment on lines +102 to +103
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the cmake installation not performing this step? How come?


def package_info(self):
self.cpp_info.set_property("cmake_find_mode", "both")
self.cpp_info.set_property("cmake_file_name", "MAVSDK")
self.cpp_info.set_property("cmake_target_name", "MAVSDK::MAVSDK")

if self.settings.os == "Linux":
self.cpp_info.system_libs = ["m", "dl", "pthread"]
if self.settings.os == "Windows":
self.cpp_info.system_libs = ["ws2_32"]

self.cpp_info.libs = [
"mavsdk",
"mavsdk_action",
"mavsdk_calibration",
"mavsdk_camera",
"mavsdk_failure",
"mavsdk_follow_me",
"mavsdk_ftp",
"mavsdk_geofence",
"mavsdk_gimbal",
"mavsdk_info",
"mavsdk_log_files",
"mavsdk_manual_control",
"mavsdk_mavlink_passthrough",
"mavsdk_mission",
"mavsdk_mission_raw",
"mavsdk_mocap",
"mavsdk_offboard",
"mavsdk_param",
"mavsdk_server_utility",
"mavsdk_shell",
"mavsdk_telemetry",
"mavsdk_tracking_server",
"mavsdk_transponder",
"mavsdk_tune"
]

self.cpp_info.includedirs.extend([
"include/mavsdk",
"include/mavsdk/plugins/action",
"include/mavsdk/plugins/calibration",
"include/mavsdk/plugins/camera",
"include/mavsdk/plugins/failure",
"include/mavsdk/plugins/follow_me",
"include/mavsdk/plugins/ftp",
"include/mavsdk/plugins/geofence",
"include/mavsdk/plugins/gimbal",
"include/mavsdk/plugins/info",
"include/mavsdk/plugins/log_files",
"include/mavsdk/plugins/manual_control",
"include/mavsdk/plugins/mavlink_passthrough",
"include/mavsdk/plugins/mission",
"include/mavsdk/plugins/mission_raw",
"include/mavsdk/plugins/mocap",
"include/mavsdk/plugins/offboard",
"include/mavsdk/plugins/param",
"include/mavsdk/plugins/server_utility",
"include/mavsdk/plugins/shell",
"include/mavsdk/plugins/telemetry",
"include/mavsdk/plugins/tracking_server",
"include/mavsdk/plugins/transponder",
"include/mavsdk/plugins/tune",
])
7 changes: 7 additions & 0 deletions recipes/mavsdk/all/test_package/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
cmake_minimum_required(VERSION 3.15)
project(test_package CXX)

find_package(MAVSDK REQUIRED)

add_executable(test_pkg test_package.cpp)
target_link_libraries(test_pkg PRIVATE MAVSDK::MAVSDK)
26 changes: 26 additions & 0 deletions recipes/mavsdk/all/test_package/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
from os import path
from conan import ConanFile
from conan.tools.build import can_run
from conan.tools.cmake import cmake_layout, CMake


class MavsdkTestConan(ConanFile):
settings = "os", "compiler", "build_type", "arch"
generators = "CMakeDeps", "CMakeToolchain", "VirtualRunEnv"
test_type = "explicit"

def requirements(self):
self.requires(self.tested_reference_str)

def layout(self):
cmake_layout(self)

def build(self):
cmake = CMake(self)
cmake.configure()
cmake.build()

def test(self):
if can_run(self):
bin_path = path.join(self.cpp.build.bindirs[0], "test_pkg")
self.run(bin_path, env="conanrun")
8 changes: 8 additions & 0 deletions recipes/mavsdk/all/test_package/test_package.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#include <mavsdk.h>

int main()
{
mavsdk::Mavsdk mavsdk;
std::string version = mavsdk.version();
Comment on lines +1 to +6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#include <mavsdk.h>
int main()
{
mavsdk::Mavsdk mavsdk;
std::string version = mavsdk.version();
#include <mavsdk.h>
#include <iostream>
int main()
{
mavsdk::Mavsdk mavsdk;
std::string version = mavsdk.version();
std::cout << "MAVSDK version: " << version << std::endl;

To ensure the compiler does not optimize these calls away

return 0;
}