search

ros2 / rosbag2_bag_v2

rosbag2 plugin for replaying ros1 version2 bag files
Apache License 2.0
24 stars 4 forks source link

Fix build for latest rosbag2 StorageOptions API #38

Closed emersonknapp closed 3 years ago

emersonknapp commented 3 years ago

Fixes #37

emersonknapp commented 3 years ago

As per usual, the only builds that have a ROS 1 environment to test against are the packaging builds -

Packaging Linux: Build Status

Karsten1987 commented 3 years ago

Build Status

emersonknapp commented 3 years ago

Thanks for running that build - I had forgotten which fields to fill and was looking through old PRs for an example

Karsten1987 commented 3 years ago

Not really sure why this test is failing:

12:57:34 3: [==========] Running 3 tests from 1 test suite.
12:57:34 3: [----------] Global test environment set-up.
12:57:34 3: [----------] 3 tests from PlayEndToEndTestFixture
12:57:34 3: [ RUN      ] PlayEndToEndTestFixture.play_all
12:57:34 3: [ERROR] [1610492254.934685691] [rosbag2_cpp]: Requested converter for format 'cdr' does not exist
12:57:34 3: [ERROR] [1610492254.935950959] [rosbag2_transport]: Failed to play: Could not find converter for format cdr
12:57:34 3: storage_options uri /home/jenkins-agent/workspace/packaging_linux/ws/src/ros2/rosbag2_bag_v2/rosbag2_bag_v2_plugins/resources/test_bag_end_to_end.bag
12:57:34 3: [       OK ] PlayEndToEndTestFixture.play_all (79 ms)
12:57:34 3: [ RUN      ] PlayEndToEndTestFixture.play_end_to_end_test_does_not_try_to_publish_ros1_topics
12:57:35 3: [ERROR] [1610492255.305204726] [rosbag2_cpp]: Requested converter for format 'cdr' does not exist
12:57:35 3: [ERROR] [1610492255.305302471] [rosbag2_transport]: Failed to play: Could not find converter for format cdr
12:58:34  3/12 Test  #3: test_rosbag2_play_rosbag_v2_end_to_end ...***Timeout  60.03 sec

is there a test-dependency missing?

emersonknapp commented 3 years ago

Huh - that is confusing - the tests are passing locally but i'm testing against packages rosdep installed from Rolling, rather than a full source build - I can try that next.

is there a test-dependency missing?

I don't think so

rosbag2_bag_v2_plugins ---> <depend>rosbag2</depend>
rosbag2 ---> <test_depend>rosbag2_tests</test_depend>  <exec_depend>rosbag2_converter_default_plugins</exec_depend>
rosbag2_tests --->  <test_depend>rosbag2_converter_default_plugins</test_depend>
rosbag2_converter_default_plugins --->   <depend>rmw_fastrtps_cpp</depend>
rmw_fastrtps_cpp -->   <build_depend>fastcdr</build_depend>
emersonknapp commented 3 years ago

Oh - from https://ci.ros2.org/view/packaging/job/packaging_linux/2154/consoleFull#console-section-13

14:00:23 Create marker file: src/ros2/rosidl_typesupport_connext/connext_cmake_module/COLCON_IGNORE
14:00:23 Create marker file: src/eProsima/Fast-CDR/COLCON_IGNORE
14:00:23 Create marker file: src/eProsima/Fast-DDS/COLCON_IGNORE
14:00:23 Create marker file: src/ros2/rosidl_typesupport_fastrtps/fastrtps_cmake_module/COLCON_IGNORE
14:00:23 Create marker file: src/ros2/rmw_connext/rmw_connext_cpp/COLCON_IGNORE
14:00:23 Create marker file: src/ros2/rmw_connext/rmw_connext_shared_cpp/COLCON_IGNORE
14:00:23 Create marker file: src/ros2/rmw_fastrtps/rmw_fastrtps_cpp/COLCON_IGNORE
14:00:23 Create marker file: src/ros2/rmw_fastrtps/rmw_fastrtps_dynamic_cpp/COLCON_IGNORE
14:00:23 Create marker file: src/ros2/rmw_fastrtps/rmw_fastrtps_shared_cpp/COLCON_IGNORE
14:00:23 Create marker file: src/ros2/rosidl_typesupport_connext/rosidl_typesupport_connext_c/COLCON_IGNORE
14:00:23 Create marker file: src/ros2/rosidl_typesupport_connext/rosidl_typesupport_connext_cpp/COLCON_IGNORE
14:00:23 Create marker file: src/ros2/rosidl_typesupport_fastrtps/rosidl_typesupport_fastrtps_c/COLCON_IGNORE
14:00:23 Create marker file: src/ros2/rosidl_typesupport_fastrtps/rosidl_typesupport_fastrtps_cpp/COLCON_IGNORE
emersonknapp commented 3 years ago

I think we just needed to check the "USE_FASTRTPS" box, which is unchecked by default now

Build Status

emersonknapp commented 3 years ago

The tests spammed megabytes worth of output 

[rosbag2_transport]: Message queue starved. Messages will be delayed. Consider increasing the --read-ahead-queue-size option.

PlayOptions::read_ahead_queue_size is the one uninitialized value in that struct, so I'm hypothesizing that it's evaluating in this environment to something unusable, not entirely sure though. I've initialized the value to the default of 1000, running tests again

Build Status

emersonknapp commented 3 years ago

Woohoo! There only remains one warning - and it is from the ROS 1 source code that gets pulled in out-of-tree. I believe this is expected.

Karsten1987 commented 3 years ago

thanks for catching up with it!

Karsten1987 commented 3 years ago

I think we just needed to check the "USE_FASTRTPS" box, which is unchecked by default now

This goes back to our discussion about what to do with switching to CycloneDDS. I think it makes sense to solely depend on fastcdr to avoid these type of problems in the future.

emersonknapp commented 3 years ago

This goes back to our discussion about what to do with switching to CycloneDDS. I think it makes sense to solely depend on fastcdr to avoid these type of problems in the future.

Yes, I have that on my personal backlog to tackle in time for G-turtle release - unless somebody else gets to it before me.

However, that won't avoid this specific problem, as the CI scripts specifically ignore fastcdr as part of the fastdds set

14:00:23 Create marker file: src/eProsima/Fast-CDR/COLCON_IGNORE

I've opened a PR https://github.com/ros2/ci/pull/534 to try and address it.

Should we be blocking merging on this?