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

Fix --topics flag for ros2 bag play being ignored for all bags after the first one. #619

Merged
merged 2 commits into from
Feb 12, 2021
Merged

Fix --topics flag for ros2 bag play being ignored for all bags after the first one. #619

merged 2 commits into from
Feb 12, 2021

Conversation

hexonxons
Copy link
Contributor

@hexonxons hexonxons commented Jan 27, 2021

How to reproduce:

We need to have some splitted log. I can't provide our own log, but I think any log with any messages recorded for 2 topics will be good.

File structure of our log looks like:

metadata.yaml      rosbag_12.db3      rosbag_15.db3-shm  rosbag_18.db3-wal  rosbag_21.db3      rosbag_24.db3-shm  rosbag_27.db3-wal  rosbag_30.db3      rosbag_3.db3-shm  rosbag_6.db3-wal
rosbag_0.db3       rosbag_12.db3-shm  rosbag_15.db3-wal  rosbag_19.db3      rosbag_21.db3-shm  rosbag_24.db3-wal  rosbag_28.db3      rosbag_30.db3-shm  rosbag_3.db3-wal  rosbag_7.db3
rosbag_0.db3-shm   rosbag_12.db3-wal  rosbag_16.db3      rosbag_19.db3-shm  rosbag_21.db3-wal  rosbag_25.db3      rosbag_28.db3-shm  rosbag_30.db3-wal  rosbag_4.db3      rosbag_7.db3-shm
rosbag_0.db3-wal   rosbag_13.db3      rosbag_16.db3-shm  rosbag_19.db3-wal  rosbag_22.db3      rosbag_25.db3-shm  rosbag_28.db3-wal  rosbag_31.db3      rosbag_4.db3-shm  rosbag_7.db3-wal
rosbag_10.db3      rosbag_13.db3-shm  rosbag_16.db3-wal  rosbag_1.db3       rosbag_22.db3-shm  rosbag_25.db3-wal  rosbag_29.db3      rosbag_31.db3-shm  rosbag_4.db3-wal  rosbag_8.db3
rosbag_10.db3-shm  rosbag_13.db3-wal  rosbag_17.db3      rosbag_1.db3-shm   rosbag_22.db3-wal  rosbag_26.db3      rosbag_29.db3-shm  rosbag_31.db3-wal  rosbag_5.db3      rosbag_8.db3-shm
rosbag_10.db3-wal  rosbag_14.db3      rosbag_17.db3-shm  rosbag_1.db3-wal   rosbag_23.db3      rosbag_26.db3-shm  rosbag_29.db3-wal  rosbag_32.db3      rosbag_5.db3-shm  rosbag_8.db3-wal
rosbag_11.db3      rosbag_14.db3-shm  rosbag_17.db3-wal  rosbag_20.db3      rosbag_23.db3-shm  rosbag_26.db3-wal  rosbag_2.db3       rosbag_32.db3-shm  rosbag_5.db3-wal  rosbag_9.db3
rosbag_11.db3-shm  rosbag_14.db3-wal  rosbag_18.db3      rosbag_20.db3-shm  rosbag_23.db3-wal  rosbag_27.db3      rosbag_2.db3-shm   rosbag_32.db3-wal  rosbag_6.db3      rosbag_9.db3-shm
rosbag_11.db3-wal  rosbag_15.db3      rosbag_18.db3-shm  rosbag_20.db3-wal  rosbag_24.db3      rosbag_27.db3-shm  rosbag_2.db3-wal   rosbag_3.db3       rosbag_6.db3-shm  rosbag_9.db3-wal

For example there are 2 topics recorded: /topic1 and /topic2.
According to code I need to call ros2 bag play --topics /topic1 to play messages only from /topic1. So I've started playing in 1st terminal and start echoing messages in second one, like ros2 topic echo /topic2

In 1st terminal I got message something like

[INFO] [1611763978.658416823] [rosbag2_storage]: Opened database './rosbag_0.db3' for READ_ONLY.
[INFO] [1611763978.708594529] [rosbag2_storage]: Opened database './rosbag_1.db3' for READ_ONLY.

And when messages from rosbag_1.db3 starts playing I've got messages in 2nd terminal. But in 2nd terminal I was listening for /topic2 messages!

@hexonxons hexonxons requested a review from a team as a code owner January 27, 2021 14:19
@hexonxons hexonxons requested review from thomas-moulard and emersonknapp and removed request for a team January 27, 2021 14:19
…the first one.

Signed-off-by: Aleksandr Rozhdestvenskii <hexonxons@gmail.com>
@Karsten1987
Copy link
Collaborator

thanks for the patch. Do you mind filling out the PR description a little bit more with details on how to reproduce this? I'd like to see whether we could come up with a regression test for this.

@hexonxons
Copy link
Contributor Author

@Karsten1987 Done

@emersonknapp
Copy link
Collaborator

Yeah, this looks good, but I would also like to see a regression test

@hexonxons
Copy link
Contributor Author

@emersonknapp @Karsten1987 regression test is in the branch :)

Signed-off-by: UsatiyNyan <kirillarionov123456@gmail.com>
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.

LGTM - thanks for the patch

@emersonknapp
Copy link
Collaborator

Gist: https://gist.githubusercontent.com/emersonknapp/c8a36ec75da0a143fa208c2b298e9555/raw/be1708f6da83149c92388f5c3f573c84871a57c2/ros2.repos
BUILD args: --packages-up-to rosbag2_cpp rosbag2_tests
TEST args: --packages-select rosbag2_cpp rosbag2_tests
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/7635

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

@hexonxons
Copy link
Contributor Author

Could you also release this fix for foxy?

@emersonknapp emersonknapp merged commit ffb73b1 into ros2:master Feb 12, 2021
@emersonknapp
Copy link
Collaborator

emersonknapp commented Feb 12, 2021

If you open up a backport PR (cherry-pick the squashed commit from master onto foxy branch and open PR against foxy branch - then I'll be happy to review and run CI for that.

See #653 for an example

emersonknapp pushed a commit that referenced this pull request Feb 16, 2021
… all bags after the first one (#619) (#654)

* Fix --topics flag for ros2 bag play being ignored for all bags after the first one. (#619)

Signed-off-by: Aleksandr Rozhdestvenskii <hexonxons@gmail.com>
Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Add windows include guard around new test

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

Co-authored-by: Emerson Knapp <eknapp@amazon.com>
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.

None yet

4 participants