rosbag2_cpp: move local message definition source out of MCAP plugin #1265

james-rms · 2023-03-19T19:31:39Z

The intention of this PR is to move the message-definition-finding capability outside of rosbag2_storage_mcap, and allow any rosbag2 storage plugin to store message definitions.

rosbag2_cpp/src/rosbag2_cpp/writers/sequential_writer.cpp

rosbag2_storage/include/rosbag2_storage/storage_interfaces/base_write_interface.hpp

Signed-off-by: James Smith <james@foxglove.dev>

rosbag2_cpp/include/rosbag2_cpp/message_definitions/message_definition_cache.hpp

rosbag2_storage_mcap/src/mcap_storage.cpp

- Also added dummy initializers for `type_hash` in TopicMetadata to address compile time warnings. Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

Signed-off-by: James Smith <james@foxglove.dev>

MichaelOrlov · 2023-03-31T21:06:27Z

@james-rms I noticed that rosbag2_compression start failing and pretty consistently.
Although there are not to much details from the log about reasoning.

023-03-31T19:59:04.3603209Z The following tests FAILED:
2023-03-31T19:59:04.3603574Z 	  4 - test_sequential_compression_writer (Failed)
2023-03-31T19:59:04.3603840Z Errors while running CTest
2023-03-31T19:59:04.3604226Z Output from these tests are in: /__w/rosbag2/rosbag2/ros_ws/build/rosbag2_compression/Testing/Temporary/LastTest.log
2023-03-31T19:59:04.3604762Z Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.
2023-03-31T19:59:04.3605088Z ---
2023-03-31T19:59:04.3605348Z Finished <<< rosbag2_compression [5.92s]	[ with test failures ]
2023-03-31T19:59:04.3605666Z Starting >>> rosbag2_compression_zstd
2023-03-31T19:59:04.3605979Z --- stderr: rosbag2_compression
2023-03-31T19:59:04.3606222Z Errors while running CTest
2023-03-31T19:59:04.3606600Z Output from these tests are in: /__w/rosbag2/rosbag2/ros_ws/build/rosbag2_compression/Testing/Temporary/LastTest.log
2023-03-31T19:59:04.3607472Z Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.

Unfortunately I was not able to reproduce this failure locally.
I have a suspicious that it might be related to the missing
MOCK_METHOD1(create_topic, void(const rosbag2_storage::TopicMetadata &)); in

rosbag2/rosbag2_compression/test/rosbag2_compression/mock_storage.hpp

Lines 36 to 40 in 4e87dd1

    
           MOCK_METHOD1(update_metadata, void(const rosbag2_storage::BagMetadata &)); 
        
           MOCK_METHOD2( 
        
             create_topic, void(const rosbag2_storage::TopicMetadata &, 
        
             const rosbag2_storage::MessageDefinition &)); 
        
           MOCK_METHOD1(remove_topic, void(const rosbag2_storage::TopicMetadata &));

which it had before.
Could you please try to add this mock method for the original create_topic(tm) API?

james-rms · 2023-04-01T02:47:46Z

@MichaelOrlov that can't be it, the rosbag2_storage write API doesn't have a single-argument create_topic(tm) method. Only the Writer API does.

james-rms · 2023-04-01T02:50:14Z

Gist: https://gist.githubusercontent.com/james-rms/acca7459da55f451285325a2104aae7d/raw/c0b2ed4d5edd741dc2b2004f8c9cd99ef142890d/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
TEST args: --packages-above rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11732

Linux
Linux-aarch64
Windows

MichaelOrlov · 2023-04-01T21:50:45Z

@james-rms You are right "the rosbag2_storage write API doesn't have a single-argument create_topic(tm) method"
However It seems I figured out why tests was failing. See my fix proposal and description in #1271

Signed-off-by: James Smith <james@foxglove.dev>

james-rms · 2023-04-03T03:01:45Z

Gist: https://gist.githubusercontent.com/james-rms/1da96a28cbb84b065c9be1628fb017fa/raw/c0b2ed4d5edd741dc2b2004f8c9cd99ef142890d/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
TEST args: --packages-above rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11741

Linux
Linux-aarch64
Windows

MichaelOrlov · 2023-04-03T06:19:16Z

@james-rms There are weird failure on windows with message

 Building Custom Rule C:/ci/ws/src/ros2/rosbag2/rosbag2_storage_mcap/CMakeLists.txt

  test_mcap_storage.cpp

LINK : fatal error LNK1181: cannot open input file 'Release\rosbag2_storage_mcap.lib' [C:\ci\ws\build\rosbag2_storage_mcap\test_mcap_storage.vcxproj]

---
Failed   <<< rosbag2_storage_mcap [1min 3s, exited with code 1]

Which need to investigate.

james-rms · 2023-04-04T23:01:35Z

Gist: https://gist.githubusercontent.com/james-rms/48357b3e104237c2ca27b86257e26b56/raw/c0b2ed4d5edd741dc2b2004f8c9cd99ef142890d/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
TEST args: --packages-above rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11779

Linux
Linux-aarch64
Windows

james-rms · 2023-04-05T00:18:08Z

Gist: https://gist.githubusercontent.com/james-rms/ed3b67fc54cfec8020eef1d94432724e/raw/c0b2ed4d5edd741dc2b2004f8c9cd99ef142890d/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
TEST args: --packages-above rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11783

Linux
Linux-aarch64
Windows

james-rms · 2023-04-05T02:40:55Z

Gist: https://gist.githubusercontent.com/james-rms/18f3874492d72c1f4b7d045b0bfebe7f/raw/c0b2ed4d5edd741dc2b2004f8c9cd99ef142890d/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
TEST args: --packages-above rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11784

Linux
Linux-aarch64
Windows

james-rms · 2023-04-05T04:54:34Z

Gist: https://gist.githubusercontent.com/james-rms/a55fb2b413f67ef7ae02a13c5c59ec5e/raw/c0b2ed4d5edd741dc2b2004f8c9cd99ef142890d/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
TEST args: --packages-above rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11787

Linux
Linux-aarch64
Windows

james-rms · 2023-04-05T23:26:40Z

Gist: https://gist.githubusercontent.com/james-rms/b00cebebac10d81e6c7e4abc14fe5075/raw/c0b2ed4d5edd741dc2b2004f8c9cd99ef142890d/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
TEST args: --packages-above rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11799

Linux
Linux-aarch64
Windows

Signed-off-by: James Smith <james@foxglove.dev>

james-rms · 2023-04-06T04:57:38Z

Gist: https://gist.githubusercontent.com/james-rms/73e2d46529262ec2bd0444083b97adbe/raw/c0b2ed4d5edd741dc2b2004f8c9cd99ef142890d/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
TEST args: --packages-above rosbag2_cpp rosbag2_transport rosbag2_storage_mcap rosbag2_test_msgdefs rosbag2_compression rosbag2_storage
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11803

Linux
Linux-aarch64
Windows

ros-discourse · 2023-04-24T20:57:59Z

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-4-20-2023/31087/1

james-rms changed the title ~~DRAFT:nmove local message definition cache into package~~ DRAFT: move local message definition cache into package Mar 20, 2023

james-rms commented Mar 20, 2023

View reviewed changes

rosbag2_cpp/src/rosbag2_cpp/writers/sequential_writer.cpp Outdated Show resolved Hide resolved

emersonknapp reviewed Mar 20, 2023

View reviewed changes

rosbag2_storage/include/rosbag2_storage/storage_interfaces/base_write_interface.hpp Outdated Show resolved Hide resolved

pull message definition cache

8d4838f

Signed-off-by: James Smith <james@foxglove.dev>

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch from f339275 to 8d4838f Compare March 21, 2023 05:13

james-rms added 2 commits March 21, 2023 17:57

clang-formatting

8eff6ec

Signed-off-by: James Smith <james@foxglove.dev>

uncrustify again

b00af9d

Signed-off-by: James Smith <james@foxglove.dev>

MichaelOrlov reviewed Mar 21, 2023

View reviewed changes

rosbag2_cpp/include/rosbag2_cpp/message_definitions/message_definition_cache.hpp Outdated Show resolved Hide resolved

MichaelOrlov reviewed Mar 21, 2023

View reviewed changes

rosbag2_storage_mcap/src/mcap_storage.cpp Outdated Show resolved Hide resolved

MichaelOrlov and others added 2 commits March 23, 2023 12:37

Add message_definition to create_topic(..) API

b4ed9b7

- Also added dummy initializers for `type_hash` in TopicMetadata to address compile time warnings. Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

make encoding a string

11c668b

Signed-off-by: James Smith <james@foxglove.dev>

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch from 158d081 to 11c668b Compare March 24, 2023 05:08

fix rosbag2_py

06961dc

Signed-off-by: James Smith <james@foxglove.dev>

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch 2 times, most recently from 5043a59 to 3f6d7ab Compare March 27, 2023 03:01

rename name to type_name

9b52bbe

Signed-off-by: James Smith <james@foxglove.dev>

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch from 3f6d7ab to 9b52bbe Compare March 27, 2023 03:09

rename message definition cache

db7e8d6

Signed-off-by: James Smith <james@foxglove.dev>

james-rms marked this pull request as ready for review March 27, 2023 05:35

james-rms requested a review from a team as a code owner March 27, 2023 05:35

james-rms requested review from gbiggs and hidmic and removed request for a team March 27, 2023 05:35

james-rms changed the title ~~DRAFT: move local message definition cache into package~~ rosbag2_cpp: move local message definition source out of MCAP plugin Mar 27, 2023

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch from e907b25 to d906cde Compare March 27, 2023 06:53

rosbag2_cpp: rename cache to source

7694f2b

Signed-off-by: James Smith <james@foxglove.dev>

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch from d906cde to 7694f2b Compare March 27, 2023 08:56

james-rms added 3 commits March 27, 2023 20:28

sqlite_storage: remove unused parameter name

ab3ee1d

Signed-off-by: James Smith <james@foxglove.dev>

rosbag2_storage: add type description hash to topic metadata

ede24ef

Signed-off-by: James Smith <james@foxglove.dev>

sequential_writer: write definitions once per topic

c3952ea

Signed-off-by: James Smith <james@foxglove.dev>

MichaelOrlov mentioned this pull request Apr 1, 2023

Fix for failing test_sequential_compression_writer tests #1271

Closed

rosbag2_compression: include implicit dependency on test_msgs

bac0eac

Signed-off-by: James Smith <james@foxglove.dev>

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch from 35ea31b to bac0eac Compare April 3, 2023 00:37

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch 3 times, most recently from ed65654 to be08259 Compare April 4, 2023 22:49

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch from be08259 to 1310803 Compare April 5, 2023 00:12

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch from 1310803 to 6e4791b Compare April 5, 2023 02:31

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch from 6e4791b to 6e530f3 Compare April 5, 2023 04:06

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch from 6e530f3 to 04aaeba Compare April 5, 2023 23:10

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch 3 times, most recently from 79469e1 to ad54c5b Compare April 6, 2023 04:07

make storage plugin public

c3958a6

Signed-off-by: James Smith <james@foxglove.dev>

james-rms force-pushed the james/fg-1413-rosbag2-move-message-definition-scraper branch from ad54c5b to c3958a6 Compare April 6, 2023 04:32

james-rms merged commit ccdaf17 into rolling Apr 7, 2023

delete-merged-branch bot deleted the james/fg-1413-rosbag2-move-message-definition-scraper branch April 7, 2023 00:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rosbag2_cpp: move local message definition source out of MCAP plugin #1265

rosbag2_cpp: move local message definition source out of MCAP plugin #1265

james-rms commented Mar 19, 2023

MichaelOrlov commented Mar 31, 2023 •

edited

Loading

james-rms commented Apr 1, 2023

james-rms commented Apr 1, 2023

MichaelOrlov commented Apr 1, 2023

james-rms commented Apr 3, 2023

MichaelOrlov commented Apr 3, 2023

james-rms commented Apr 4, 2023

james-rms commented Apr 5, 2023

james-rms commented Apr 5, 2023

james-rms commented Apr 5, 2023

james-rms commented Apr 5, 2023

james-rms commented Apr 6, 2023

ros-discourse commented Apr 24, 2023

rosbag2_cpp: move local message definition source out of MCAP plugin #1265

rosbag2_cpp: move local message definition source out of MCAP plugin #1265

Conversation

james-rms commented Mar 19, 2023

MichaelOrlov commented Mar 31, 2023 • edited Loading

james-rms commented Apr 1, 2023

james-rms commented Apr 1, 2023

MichaelOrlov commented Apr 1, 2023

james-rms commented Apr 3, 2023

MichaelOrlov commented Apr 3, 2023

james-rms commented Apr 4, 2023

james-rms commented Apr 5, 2023

james-rms commented Apr 5, 2023

james-rms commented Apr 5, 2023

james-rms commented Apr 5, 2023

james-rms commented Apr 6, 2023

ros-discourse commented Apr 24, 2023

MichaelOrlov commented Mar 31, 2023 •

edited

Loading