Use converters when recording a bag file #57
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
botteroa-si
force-pushed
the
feature/use_converters_in_writer
branch
from
0716186
to
eeff3fc
Compare
|
Fixes #16 |
anhosi
force-pushed
the
feature/use_converters_in_writer
branch
from
eeff3fc
to
e479b2b
Compare
|
CI
|
Martin-Idel-SI
force-pushed
the
feature/use_converters_in_writer
branch
from
e479b2b
to
f0eb3ab
Compare
|
This PR is now based on #64 - after this PR is merged, the number of files changed will shrink to 63. The changeset still comprises a lot of files since we change the behaviour of the storage plugin: Instead of writing one topic type per storage storage, now every topic has its own type. This needs to be changed in the sqlite3 backend all the way to the printing statements. |
- Use converters in Writer::write() when input rmw serialization format is different from desired storage serialization format - Add new field in rosbag2::StorageOptions to keep track of the rmw format given by the user to store the message in
- Add 'serialization_format' field to TopicMetadata - Add 'serialization_forat' column in 'topics' table in sqlite storage - Remove 'storage_format' from BagMetadata and use the TopicMetadata field directly, instead - the field 'rmw_serialization_format' has been moved from rosbag2::StorageOptions to rosbag2_transport::RecordOptions, because it's a topic property rather than a storage one. - Currently all topics in a bag file must have the same serialization format - The tests have been updated accordingly
…rter plugin does not exist
… writer's destructor
- This assures that if one of the converter plugins does not exist, the database is not created
…r plugins do not exists - Both a test for record and play has been added
…specified at CLI level - Tests to check that the serialization format is written in the database have also been added.
- update pluginlib descriptions file after several renames - fix export of missing includes folder
- one of the main test goals can only be ssen by valgrind or sanitizers - enable leak sanitizer for gcc builds only (for now)
N.B. This exposes an pre-existing memory leak (not fixed here).
- topic_name member needs to be freed - provide a setter for convenience - Directly assigning a string literal in the test is not sufficient as this would be static memory that does not need to be freed.
This allows manual usage of valgrind.
anhosi
force-pushed
the
feature/use_converters_in_writer
branch
from
f0eb3ab
to
aa73556
Compare
|
New CI after rebase
|
Karsten1987
reviewed
Nov 27, 2018
Karsten1987
approved these changes
Nov 29, 2018
| @@ -25,6 +25,7 @@ struct RecordOptions | |||
| public: | |||
| bool all; | |||
| std::vector<std::string> topics; | |||
| std::string rmw_serialization_format; | |||
There was a problem hiding this comment.
shouldn't this be a vector of strings? Ideally, each topic is associated with a serialization format (even if it's not supported yet).
There was a problem hiding this comment.
I would do this only when this feature is being implemented. I am not yet sure how this should look like (or even how the cli usage should be). E.g. a map might also be an alternative.
dirk-thomas
reviewed
Aug 25, 2020
| target_compile_definitions(test_ros2_message PRIVATE "ROSBAG2_BUILDING_DLL") | ||
| if(NOT DISABLE_SANITIZERS) | ||
| target_compile_options(test_ros2_message PUBLIC $<$<CXX_COMPILER_ID:GNU>:-fsanitize=leak>) | ||
| target_link_libraries(test_ros2_message $<$<CXX_COMPILER_ID:GNU>:-fsanitize=leak>) |
There was a problem hiding this comment.
Just for the record: in the Jenkins CI jobs which are using Docker the leak sanitizer was never used for the test. See ros-infrastructure/ros_buildfarm#832.
james-rms
added a commit
to james-rms/rosbag2
that referenced
this pull request
Nov 17, 2022
* rosbag2_storage_mcap: add storage preset profiles Signed-off-by: James Smith <james@foxglove.dev> * mcap_vendor: update to mcap v0.5.0 Signed-off-by: James Smith <james@foxglove.dev> Signed-off-by: James Smith <james@foxglove.dev>
james-rms
added a commit
to james-rms/rosbag2
that referenced
this pull request
Nov 17, 2022
* rosbag2_storage_mcap: add storage preset profiles Signed-off-by: James Smith <james@foxglove.dev> * mcap_vendor: update to mcap v0.5.0 Signed-off-by: James Smith <james@foxglove.dev> Signed-off-by: James Smith <james@foxglove.dev>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is based on #56. It includes:
-f, --serialization-formatfor the CLI verbrecordserialization-formatoption defaults to thermwformat of the incoming messagesrmwserialization format (for the moment, though, a bag file can be played only if all topics have the same format. Since it is not yet possible to edit a bag file, this is not a problem. The possibility to play a bag with different serialization formats will be added in a followup PR)