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

Overhaul in the rosbag2_transport::TopicFilter class and relevant tests #1585

Merged
merged 4 commits into from
Mar 15, 2024

Conversation

MichaelOrlov
Copy link
Contributor

@MichaelOrlov MichaelOrlov commented Mar 14, 2024

  • Follow up on the Filter topic by type  #1577

  • Cleanup in topic_filter.cpp and test. Also, add test coverage for the topic_types filter options.

  • Delete test_record_topic_types.cpp
    Rationale:

    1. Need to avoid using pub/sub and transport layer for testing basic
      functionalities is it could be avoided.
    2. The test coverage for functionality for filtering messages by topic types was added
      to the test_topic_filter.cpp in the scope of this PR.
  • Add missed --topic-types to the error message in the record.py

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
- Before we were using RegexFixture. However, many tests under this test
fixture unrelated to the regex. It will be more appropriate and to be
consistent to use one test fixture with more generic name.

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
- Rationale:
1. Need to avoid using pub/sub and transport layer for testing basic
functionalities is it could be avoided.
2. The use cases with filtering messages by topic types already covered
in the test_topic_filter.cpp

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov MichaelOrlov changed the title Overhaul in the rosbag2_transport topic filter and tests Overhaul in the rosbag2_transport::TopicFilter class and relevant tests Mar 14, 2024
@MichaelOrlov MichaelOrlov marked this pull request as ready for review March 14, 2024 19:13
@MichaelOrlov MichaelOrlov requested a review from a team as a code owner March 14, 2024 19:13
@MichaelOrlov MichaelOrlov requested review from emersonknapp, hidmic and ahcorde and removed request for a team March 14, 2024 19:13
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

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

@ahcorde
Copy link
Contributor

ahcorde commented Mar 14, 2024

@MichaelOrlov CI is green, feel free to merge it and then I can rebase the other PR

@MichaelOrlov
Copy link
Contributor Author

@ahcorde Thanks for a quick review.

@MichaelOrlov MichaelOrlov merged commit e5d56d7 into rolling Mar 15, 2024
13 of 14 checks passed
@delete-merged-branch delete-merged-branch bot deleted the morlov/overhaul_in_topic_filter branch March 15, 2024 03:02
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

2 participants