Add Fast dds #5968

dotChris90 · 2021-06-18T13:20:47Z

Specify library name and version: fast-dds/2.3.2 (also known as fastrtps/2.3.2)

Fast-DDS is an implementation of the DDS standard for IoT devices.
DDS is an awesome pub/sub mechanism which has a lot of time critical important properties for industry.
I didnt find so far a DDS implementation in CCI - that has to change now :)

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

prince-chrismc

Can you please shrink the test_package? seems like a lot to make sure the contents are all inplace... but I am not familiar with this library

prince-chrismc · 2021-06-26T16:29:54Z

recipes/fast-dds/all/conanfile.py

+        version = tools.Version(self.settings.compiler.version)
+        if compiler.get_safe("cppstd"):
+            tools.check_min_cppstd(self, 11)
+        if os == "Linux" and compiler == "gcc" and version < "5":


We have a template conan-io/conan#8002 you can reuse to also print a warning for the consumer

got your point - i took the dictionary for C++11 from https://github.com/conan-io/conan-center-index/blob/41d4fbb8c218f9d938ccb8b9d888bff52002e62e/recipes/cxxopts/all/conanfile.py.

and btw - nice idea - to have a standard template here ;)

prince-chrismc · 2021-06-26T16:31:01Z

recipes/fast-dds/all/conanfile.py

+            self.cpp_info.components["fastrtps"].system_libs = [
+                    "pthread"
+                ]


Suggested change

self.cpp_info.components["fastrtps"].system_libs = [

"pthread"

]

self.cpp_info.components["fastrtps"].system_libs.append("pthread")

prince-chrismc · 2021-06-26T16:31:26Z

recipes/fast-dds/all/conanfile.py

+            self.cpp_info.components["fastrtps"].system_libs = [
+                    "rt",
+                    "dl",
+                    "atomic"
+                ]


Suggested change

self.cpp_info.components["fastrtps"].system_libs = [

"rt",

"dl",

"atomic"

]

self.cpp_info.components["fastrtps"].system_libs.extends(["rt", "dl", "atomic"])

prince-chrismc · 2021-06-26T16:32:46Z

recipes/fast-dds/all/CMakeLists.txt

+set(CMAKE_CXX_STANDARD 11)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+


Since you are blocking older compilers this should not be required (it also blocks consumers from setting the cppstd settings)

Suggested change

set(CMAKE_CXX_STANDARD 11)

set(CMAKE_CXX_STANDARD_REQUIRED ON)

prince-chrismc · 2021-06-26T16:34:18Z

recipes/fast-dds/all/conanfile.py

+            if ("MT" in self.settings.compiler.runtime and self.options.shared):
+                # This combination leads to an fast-dds error when linking
+                # linking dynamic '*.dll' and static MT runtime
+                raise ConanInvalidConfiguration("Mixing a dll eprosima library with a static runtime is a bad idea")


Suggested change

raise ConanInvalidConfiguration("Mixing a dll eprosima library with a static runtime is a bad idea")

raise ConanInvalidConfiguration("Mixing a dll {} library with a static runtime is a bad idea".format(self.name))

conan-center-bot · 2021-06-30T12:36:20Z

All green in build 10 (97b799770e357c70ce5a0d37a4476617530d022a):

fast-dds/2.3.2@:
All packages built successfully! (All logs)

dotChris90 · 2021-06-30T12:50:50Z

Wuhu - finally :D

SpaceIm · 2021-07-02T10:09:49Z

recipes/fast-dds/all/conanfile.py

+        self.cpp_info.components["tools"].name = "tools"
+        self.cpp_info.components["tools"].bindirs = [os.path.join("bin","tools")]


As usual, too generic for a conan component name where pkg_config name is not overriden. Anyway this component doesn't exist upstream (there are tools, but their targets are not exported).

I advice also to be explicit on the type of "names" (names["cmake_find_package"] etc, this library doesn't provide official pkgconfig files for example).

will be resolved in #6429

SpaceIm · 2021-07-02T10:28:00Z

recipes/fast-dds/all/conanfile.py

+        self.cpp_info.components["fastrtps"].build_modules["cmake_find_package"] = [self._module_file_rel_path]
+        self.cpp_info.components["fastrtps"].build_modules["cmake_find_package_multi"] = [self._module_file_rel_path]
+        # component fast-discovery
+        self.cpp_info.components["fast-discovery"].name = "fast-discovery"


it's fast-discovery-server

Oh Jesus.... Damn.... I would rise an issue and assign it to myself. And in a separate PR resolve the issues.

will be resolved in #6429

gbellizio · 2021-08-06T13:37:37Z

Hi, I tried to compile HelloWorld example program that comes with Fast-DDS source code as a standalone CMake project using the provided Conan recipe. The process crashes as soon as the publisher starts (or the subscriber, depends on which order publisher and subscriber are executed). The crashing process prints out the following on terminal:

[foonathan::memory] Allocator foonathan::memory::memory_pool (at 0x4697c8) received invalid size/alignment 56, max supported is 48terminate called after throwing an instance of 'foonathan::memory::bad_node_size'
  what():  allocation node size exceeds supported maximum of allocator

I guess the issue is due to the fact that the recipe requires foonathan-memery v 0.7.0 library, but Fast-DDS try to use a patched snapshot at commit c619113 (from https://github.com/eProsima/foonathan_memory_vendor/blob/master/CMakeLists.txt line 79)

dotChris90 · 2021-08-06T15:58:11Z

@gbellizio - thanks for trying it out - i though at the time of creation i checked that V0.7.0 has already included the patches but i can check again - the test package should validate the publishing process --> https://github.com/conan-io/conan-center-index/blob/master/recipes/fast-dds/all/test_package/test_package.cpp.

Can you give some small details like Windows or Linux build and cross build or normal? :) the normal HelloWorld - or with shared memory?

gbellizio · 2021-08-06T16:37:19Z

Hi @dotChris90 and thanks for fast reply. My build environment is Fedora Linux 34 using latest version of conan (v. 1.39.0), g++ 11. I compiled the normal version of HelloWorkd example by copying source files from Fast-DDS code base into a standalone CMake project.

By downloading Fast-DDS source code from github and building it by following instructions on project page (which makes use of patched version of foonathan-memory, https://github.com/eProsima/foonathan_memory_vendor.git) the same example successfully runs.

foonathan_memory_vendor.git downloads code form the original repository and switches to commit c619113 which is two years old.

By looking at the test package I noted that only the publisher is instantiated. The issue happens as soon as the subscriber joins.

dotChris90 · 2021-08-06T16:44:09Z

Thanks for the hint. I will try it out at weekend. :)

I thought before I was trying out publishing subscription in one process. After i was reducing to minimal example (publishing in test_package).

But I have to admit : not tried out two separated process.

One last question : which version u referencing?

gbellizio · 2021-08-06T16:49:13Z

fast-dds/2.3.3

dotChris90 · 2021-08-07T13:03:23Z

@gbellizio - i did setup on fedora and ubuntu system - both the same with 2.3.3 and 2.3.2.

yes you are right -.- building, linking everything works like expected. But on runtime - crash - all 3 examples - normal HelloWorld, shared memory and TCP. ultimate worst case -.-

I read on github some other people where facing this issue too --> foonathan/memory#123
But i can confirm that "clean workspace" does not help and it's deterministic.

I will create an issue (because this PR already merged), mention that i work on it and compare their foonathan patch with the 0.7.0 again. I took the 0.7.0 because packaging should work with versions and not commit_ids .... but maybe i was too quick ...

i keep you updated.

dotChris90 · 2021-08-10T07:02:00Z

I saw that 0.7.0 truly have more changes to 0.6.2 than i though - especially related to errors and node_sizes. i will try 0.6.2 - then with a patched 0.6.2 and then discuss with others how to handle it.

dotChris90 · 2021-08-13T10:23:34Z

@gbellizio i actually found a quick solution (sorry took long since recovery from covid vaccination ^^).

the 0.7.0 memory version will run if! i compile with the extra option self._cmake.definitions["FOONATHAN_MEMORY_CHECK_ALLOCATION_SIZE"] = False

when this tiny change was included - then i could run all three examples - normal Helloworld, tcp, shared memory.

dotChris90 · 2021-08-13T10:24:37Z

I will add this option to the memory package and fast-dds will use the option = False as long as i come up with a better solution. i maybe asking fast-dds team why they dont reference 0.7.0

gbellizio · 2021-08-23T09:58:22Z

Hi @dotChris90. I just gave it a try by disabling the size checks as you suggested. Unfortunately, runtime errors still persist, at least in debug mode:

[foonathan::memory] Buffer overflow at address 0x1d65520 detected, corresponding memory block 0x1d654f0 has only size 48.[1]    18046 IOT instruction (core dumped)  ./DDSHelloWorldExample publisher

In release mode the example program works as expected, but I'm not sure we are safe from memory allocation issues for more complex (and realistic) code:

dotChris90 · 2021-08-23T10:00:44Z

@gbellizio - thanks for pointing it out .... i will try to discuss with eprosima together best =.=

dotChris90 added 13 commits May 22, 2021 12:35

init

0b0fd98

compile success, link fails

025a4ab

idk

0617129

allow true

fb23447

add patches

76921b4

working status

53c0bba

without args

aef792d

add fix for find package in containers

d7e7088

added alias

2e2faf4

link against windows system libs

75e8e40

changed for hooks

2857abd

adapted for windows 100% run

84a3525

Add atomic as dependency

cab9911

github-actions bot mentioned this pull request Jun 18, 2021

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Jun 18, 2021

force C++11 and check gcc version

8116fdf

This comment has been minimized.

Sign in to view

fix : c++11 require cmake 3.1

7a01e15

This comment has been minimized.

Sign in to view

adapted ccompiler check

59ec565

This comment has been minimized.

Sign in to view

validate existence

ea11a79

This comment has been minimized.

Sign in to view

reduce complexity + add openssl as requires

a161e6a

This comment has been minimized.

Sign in to view

conan-center-bot removed the Failed label Jun 19, 2021

SSE4 requested a review from uilianries June 21, 2021 06:00

conan-center-bot requested a review from danimtb June 24, 2021 11:57

prince-chrismc reviewed Jun 26, 2021

View reviewed changes

dotChris90 closed this Jun 30, 2021

dotChris90 reopened this Jun 30, 2021

This comment has been minimized.

Sign in to view

conan-center-bot removed the Unexpected Error label Jun 30, 2021

Rise number to avoid partly C++11 implementations

97b7997

dotChris90 dismissed danimtb’s stale review via 97b7997 June 30, 2021 07:58

conan-center-bot removed the Failed label Jun 30, 2021

prince-chrismc approved these changes Jun 30, 2021

View reviewed changes

dotChris90 requested a review from danimtb July 1, 2021 12:49

danimtb approved these changes Jul 2, 2021

View reviewed changes

SSE4 approved these changes Jul 2, 2021

View reviewed changes

conan-center-bot merged commit ecffe66 into conan-io:master Jul 2, 2021

SpaceIm reviewed Jul 2, 2021

View reviewed changes

This was referenced Jul 2, 2021

Information : Fast-DDS 2.3.2 is now part of Conan Center eProsima/Fast-DDS#2045

Closed

Improvements in meta information - Fast-DDS/2.3.2 #6137

Closed

dotChris90 mentioned this pull request Aug 7, 2021

[package] Fast-DDS/2.3.3 and 2.3.2 : Compiled examples throwing runtime error #6731

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Fast dds #5968

Add Fast dds #5968

dotChris90 commented Jun 18, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

prince-chrismc left a comment

prince-chrismc Jun 26, 2021

dotChris90 Jun 28, 2021

dotChris90 Jun 28, 2021

prince-chrismc Jun 26, 2021

prince-chrismc Jun 26, 2021

prince-chrismc Jun 26, 2021

prince-chrismc Jun 26, 2021

This comment has been minimized.

conan-center-bot commented Jun 30, 2021

dotChris90 commented Jun 30, 2021

SpaceIm Jul 2, 2021 •

edited

Loading

dotChris90 Jul 21, 2021

SpaceIm Jul 2, 2021

dotChris90 Jul 2, 2021

dotChris90 Jul 21, 2021

gbellizio commented Aug 6, 2021

dotChris90 commented Aug 6, 2021

gbellizio commented Aug 6, 2021 •

edited

Loading

dotChris90 commented Aug 6, 2021 •

edited

Loading

gbellizio commented Aug 6, 2021

dotChris90 commented Aug 7, 2021 •

edited

Loading

dotChris90 commented Aug 10, 2021

dotChris90 commented Aug 13, 2021 •

edited

Loading

dotChris90 commented Aug 13, 2021

gbellizio commented Aug 23, 2021

dotChris90 commented Aug 23, 2021

		set(CMAKE_CXX_STANDARD 11)
		set(CMAKE_CXX_STANDARD_REQUIRED ON)

	raise ConanInvalidConfiguration("Mixing a dll eprosima library with a static runtime is a bad idea")
	raise ConanInvalidConfiguration("Mixing a dll {} library with a static runtime is a bad idea".format(self.name))

		self.cpp_info.components["tools"].name = "tools"
		self.cpp_info.components["tools"].bindirs = [os.path.join("bin","tools")]

Add Fast dds #5968

Add Fast dds #5968

Conversation

dotChris90 commented Jun 18, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

prince-chrismc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

conan-center-bot commented Jun 30, 2021

dotChris90 commented Jun 30, 2021

SpaceIm Jul 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gbellizio commented Aug 6, 2021

dotChris90 commented Aug 6, 2021

gbellizio commented Aug 6, 2021 • edited Loading

dotChris90 commented Aug 6, 2021 • edited Loading

gbellizio commented Aug 6, 2021

dotChris90 commented Aug 7, 2021 • edited Loading

dotChris90 commented Aug 10, 2021

dotChris90 commented Aug 13, 2021 • edited Loading

dotChris90 commented Aug 13, 2021

gbellizio commented Aug 23, 2021

dotChris90 commented Aug 23, 2021

SpaceIm Jul 2, 2021 •

edited

Loading

gbellizio commented Aug 6, 2021 •

edited

Loading

dotChris90 commented Aug 6, 2021 •

edited

Loading

dotChris90 commented Aug 7, 2021 •

edited

Loading

dotChris90 commented Aug 13, 2021 •

edited

Loading