Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

minimal c++ API test #451

Merged
merged 2 commits into from
Jul 10, 2020
Merged

minimal c++ API test #451

merged 2 commits into from
Jul 10, 2020

Conversation

Karsten1987
Copy link
Collaborator

That's a first step into improving the general C++ API for writing and reading rosbags.

The set of API calls in the test are the minimal set I could come up with in order to write a MessageT into the bag file. IMHO, there's quite some room for making this more user friendly.

  • Making the SequentialReader and SequentialWriter the default implementation. That would allow for rosbag2_cpp::Writer writer; to simply work.
  • Default the converter options. As seen in here and here the current converter API is rather confusing than helpful. A call to open with only specifying the StorageOptions should suffice and the converter functionality should be bypassed as such.
  • Every topic has to be created beforehand. However, if you forget that you'll get a uncaught exception such as unknown file: Failure C++ exception with description "unordered_map::at: key not found" thrown in the test body. I've addressed that in this PR.
  • Write bag messages which are allocated on the stack: There's currently no way of writing serialized data which was allocated on the stack, but has to be passed in via a shared pointer. That results in specifying a custom deleter which avoids to deallocate the data (malloc: *** error for object 0x7ffeefbfdc88: pointer being freed was not allocated).

Signed-off-by: Karsten Knese <karsten@openrobotics.org>
@Karsten1987 Karsten1987 self-assigned this Jul 2, 2020
@Karsten1987 Karsten1987 mentioned this pull request Jul 2, 2020
Copy link
Collaborator

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good project. That's a lot of lines code to just write a single message into a bagfile.

@Karsten1987
Copy link
Collaborator Author

running CI before merging:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Signed-off-by: Karsten Knese <karsten@openrobotics.org>
@Karsten1987
Copy link
Collaborator Author

GH actions are showing the linters being fixed. Merging.

@Karsten1987 Karsten1987 merged commit c06cb47 into master Jul 10, 2020
@delete-merged-branch delete-merged-branch bot deleted the minimal_cpp_test branch July 10, 2020 19:35
@dirk-thomas
Copy link
Member

The Windows CI build shows a compiler warning (https://ci.ros2.org/job/ci_windows/11319/msbuild/) which is also present in the last nightly builds.

@ros-discourse
Copy link

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-2020-07-16/15468/1

emersonknapp pushed a commit that referenced this pull request Feb 2, 2021
* minimal c++ API test

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* linters

Signed-off-by: Karsten Knese <karsten@openrobotics.org>
emersonknapp pushed a commit that referenced this pull request Feb 17, 2021
* minimal c++ API test

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* linters

Signed-off-by: Karsten Knese <karsten@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants