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.

Sign up for GitHub

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

Jump to bottom

Reduce message spam when topics to be recorded do not exist #1018

Merged
merged 3 commits into from
May 25, 2022
Merged

Reduce message spam when topics to be recorded do not exist #1018

merged 3 commits into from
May 25, 2022

Conversation

ihasdapie
Copy link
Member

Commits extend lifetime of TopicFilter to avoid emitting warning messages on instantiation. topic_filter.hpp moved to make it accessible from recorder.hpp.

@ihasdapie
move topic_filter.hpp to expose to recorder.hpp
8286b40 
Signed-off-by: Brian Chen <brian.chen@openrobotics.org>
@ihasdapie ihasdapie requested a review from jacobperron May 18, 2022 23:43
@ihasdapie ihasdapie requested a review from a team as a code owner May 18, 2022 23:43
@ihasdapie ihasdapie requested review from gbiggs and hidmic and removed request for a team May 18, 2022 23:43
MichaelOrlov
MichaelOrlov requested changes May 19, 2022
View reviewed changes
Copy link
Contributor

@MichaelOrlov MichaelOrlov left a comment

Choose a reason for hiding this comment

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

@ihasdapie Thank you for your contribution.
Overall looks good among a couple nitpicks.
Please see my comments.

rosbag2_transport/include/rosbag2_transport/recorder.hpp Outdated Show resolved Hide resolved
rosbag2_transport/include/rosbag2_transport/topic_filter.hpp Outdated Show resolved Hide resolved
jacobperron
jacobperron approved these changes May 19, 2022
View reviewed changes
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM with green CI

@ihasdapie
Copy link
Member Author

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

ihasdapie added 2 commits May 22, 2022 11:44
@ihasdapie
fix #1011: persist TopicFilter to avoid warning spam
afb3321 
Signed-off-by: Brian Chen <brian.chen@openrobotics.org>
@ihasdapie
address pr comments
1f14f9d 
Signed-off-by: Brian Chen <brian.chen@openrobotics.org>
@ihasdapie ihasdapie force-pushed the brianc/fix_rosbag_msg_spam branch from 9729d86 to 1f14f9d Compare May 22, 2022 18:44
jacobperron
jacobperron approved these changes May 24, 2022
View reviewed changes
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

I think the test failures are unrelated here's the same Linux failure appearing on last night's nightly and the Windows failure appears unrelated to rosbag2.

@MichaelOrlov Could you take another look?

@MichaelOrlov
Copy link
Contributor

@jacobperron I confirm that test failures in projectroot.test_play_services__rmw_fastrtps_cpp and https://build.ros2.org/job/Rpr__rosbag2__ubuntu_jammy_amd64/93/testReport/junit/(root)/projectroot/test_play_services__rmw_fastrtps_cpp/ unrelated to the changes in this PR.

Please see my latest analysis for those failures here #1013 (comment)

@ihasdapie
Copy link
Member Author

@MichaelOrlov Can you approve this PR so we can get it merged? Thanks

@MichaelOrlov
Copy link
Contributor

@ihasdapie Sure I forgot that I requested changes and haven't approved afterwards.

@ihasdapie ihasdapie closed this May 25, 2022
@ihasdapie ihasdapie reopened this May 25, 2022
@ihasdapie ihasdapie merged commit d97e291 into master May 25, 2022
@delete-merged-branch delete-merged-branch bot deleted the brianc/fix_rosbag_msg_spam branch May 25, 2022 22:32
DanMesh pushed a commit to DanMesh/rosbag2 that referenced this pull request Aug 7, 2023
@ihasdapie @DanMesh
Reduce message spam when topics to be recorded do not exist (ros2#1018)
ac6b596 
* move topic_filter.hpp to expose to recorder.hpp

Signed-off-by: Brian Chen <brian.chen@openrobotics.org>

* fix ros2#1011: persist TopicFilter to avoid warning spam

Signed-off-by: Brian Chen <brian.chen@openrobotics.org>

* address pr comments

Signed-off-by: Brian Chen <brian.chen@openrobotics.org>
DanMesh pushed a commit to DanMesh/rosbag2 that referenced this pull request Aug 7, 2023
@ihasdapie @DanMesh
Reduce message spam when topics to be recorded do not exist (ros2#1018)
e5766c2 
* move topic_filter.hpp to expose to recorder.hpp

Signed-off-by: Brian Chen <brian.chen@openrobotics.org>

* fix ros2#1011: persist TopicFilter to avoid warning spam

Signed-off-by: Brian Chen <brian.chen@openrobotics.org>

* address pr comments

Signed-off-by: Brian Chen <brian.chen@openrobotics.org>
@DanMesh DanMesh mentioned this pull request Aug 7, 2023
Closed
@MichaelOrlov MichaelOrlov mentioned this pull request Sep 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelOrlov MichaelOrlov MichaelOrlov approved these changes

@jacobperron jacobperron jacobperron approved these changes

@gbiggs gbiggs Awaiting requested review from gbiggs gbiggs is a code owner automatically assigned from ros2/tooling-reviewers

@hidmic hidmic Awaiting requested review from hidmic hidmic is a code owner automatically assigned from ros2/tooling-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Warnings about 'Topic ... has unknown type' spam when specifying topics to be recorded which do not exist
3 participants
@ihasdapie @MichaelOrlov @jacobperron