search

ros2 / performance_test

Tool to test the performance of pub/sub based communication frameworks.
Apache License 2.0
13 stars 6 forks source link

Support of FastDDS 2.x #20

Closed richiware closed 4 years ago

richiware commented 4 years ago

Fixes #19.

ivanpauno commented 4 years ago

We're using run_on_builfarm branch, so this PR should target it.

richiware commented 4 years ago

That define is provided by Fast-DDS (fastrtps/config.h). No changes should be made.

El vie., 19 jun. 2020 21:24, Ivan Santiago Paunovic < notifications@github.com> escribió:

@ivanpauno commented on this pull request.

In performance_test/src/communication_abstractions/resource_manager.cpp https://github.com/ros2/performance_test/pull/20#discussion_r443008930:

@@ -85,7 +85,7 @@ eprosima::fastrtps::Participant * ResourceManager::fastrtps_participant() const disc_config.m_simpleEDP.use_PublicationWriterANDSubscriptionReader = true; disc_config.leaseDuration = eprosima::fastrtps::c_TimeInfinite;

endif

  • PParam.rtps.builtin.domainId = m_ec.dds_domain_id();
  • PParam.domainId = m_ec.dds_domain_id();

There's no compile option being set in the CMakeLists.txt called FASTRTPS_VERSION_MAJOR, e.g. https://github.com/ros2/performance_test/blob/ad8a90a6333ff6008fd195c3cb36ccc90e5eb2dd/performance_test/CMakeLists.txt#L61

We need some extra logic in the CMakelists.txt for this to work

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ros2/performance_test/pull/20#discussion_r443008930, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQQ5LIY6UINRMQIVZR2BQTRXO3OJANCNFSM4OALOANA .

ivanpauno commented 4 years ago

Nightly jobs are currently broken because of this. I'm not sure why they started to fail on June 19th when https://github.com/eProsima/Fast-DDS/pull/1227 was merged on May 25th (maybe, we're using released versions in Fci??).

I'm going to merge this to see if it unblocks the nightly jobs. FYI @cottsay

ivanpauno commented 4 years ago

Thanks for the PR @richiware !!

ivanpauno commented 4 years ago

Oh :man_facepalming:, I forgot this is targeting the incorrect branch: https://github.com/ros2/performance_test/pull/20#issuecomment-645351593.

I will open another PR targeting the correct one.

cottsay commented 4 years ago

I really think there should be a PR upstream as well. We really want to kill this fork.

ivanpauno commented 4 years ago

I really think there should be a PR upstream as well. We really want to kill this fork.

Yeah, I'm planning to do that too.