Skip to content

Rosbag2 py#638

Closed
agutenkunst wants to merge 2 commits intoros2:foxyfrom
agutenkunst:rosbag2_py
Closed

Rosbag2 py#638
agutenkunst wants to merge 2 commits intoros2:foxyfrom
agutenkunst:rosbag2_py

Conversation

@agutenkunst
Copy link
Copy Markdown

Considering that Foxy is supposed to stay until May 2023 I think adding rosbag2_py could help a lot of users.
This PR is primarly intented to open the discussion.

Previously discussed in: #308 (comment)

I think the opinion of @mabelzhang @jacobperron @mjeronimo @ros2/tooling-approvers is valueable here.

mabelzhang and others added 2 commits February 5, 2021 17:15
* Initialize new package rosbag2_py

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
(cherry picked from commit f2ec0b7)

* Proof-of-concept implementation using pybind11

Expose the sequential reader to iterate over messages from Python.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
(cherry picked from commit dc68894)

* pybind StorageOptions and ConverterOptions; linter fixes

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
(cherry picked from commit 91f6763)

* return timestamp

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
(cherry picked from commit deafbf8)

* simplify binding of structs

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
(cherry picked from commit da561cc)

* pybind TopicMetadata

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
(cherry picked from commit 4a4e1b8)

* namespace rosbag2 -> rosbag2_cpp

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* small fixes

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* initial pytests

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* wrap Reader instead of SequentialReader

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* small fixes for PR comments

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* change workflow to autotest feature branch

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* revert workflow

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* added writer and writer tests

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* fix test path setup

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* fix and make more rigorous writer test

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* wrapper and test for topic filter

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* linters. change pybind package

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* simplify verbose import

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* tidy up

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* explicit subclasses of Reader and Writer

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* Use template specialization instead of inheritence

This makes the implementation slightly more compact. And adding a new specialization should be more straight forward as we only have to modify the cpp file in one place.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove temporary variable

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove unused member variable

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Move to initializer list

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* refactor into reader writer storage

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* cleanup includes

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* switch back to pybind11_vendor

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* Packages dependency for CI

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* printout to debug CI test failure

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* use tempfile for writer test, add exec_depend

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* remove exec_depend

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* use pytest fixture tmp_path; add rosbag2_py to rosbag2

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* add makedirs back to see CI result

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* fix import error

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* cleanup

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* add modules explicitly for windows CI

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* cleanup

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* try more <test_depend>s for mac test failure

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* wrap structs with named arguments for Python keyword params

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* add pytest.ini

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* add rosbag db3-shm and db3-wal files for macOS test

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
Co-authored-by: Andreas Klintberg <ankl@kth.se>
Signed-off-by: Alexander Gutenkunst <alex.gutenkunst+github.com>
* AMENT_IGNORE rosbag2_py

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* remove rosbag2_py from meta package

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Alexander Gutenkunst <alex.gutenkunst+github.com>
@jacobperron
Copy link
Copy Markdown
Member

There is also #625, which is proposing a full backport master to foxy (possibly including rosbag2_py if all goes smoothly).

@agutenkunst
Copy link
Copy Markdown
Author

Do you suggest a close of this draft PR in favor of #625 or are the PRs complementary such that this PR could be merged first?

The buildfarm seems to have a warning

Warning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3, and in 3.9 it will stop working

not sure what to make of it....

@emersonknapp
Copy link
Copy Markdown
Collaborator

Do you suggest a close of this draft PR in favor of #625 or are the PRs complementary such that this PR could be merged first?

Yes, the suggestion is closing this PR in favor of #625, which contains everything currently on rosbag2:master. I'd prefer to wait until we resolve the conversation in https://discourse.ros.org/t/fast-forward-merging-rosbag2-master-api-to-foxy/18927/18 before making any major backports to Foxy (such as the full package rosbag2_py

@agutenkunst
Copy link
Copy Markdown
Author

Ok closing

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