-
Notifications
You must be signed in to change notification settings - Fork 240
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
Add per group statistics in rosbag2_performance_benchmarking #1306
Add per group statistics in rosbag2_performance_benchmarking #1306
Conversation
@carlossvg Could you please review this PR? |
f5474e0
to
4f1bf1b
Compare
400821a
to
0d8fb22
Compare
Note. I've made a fresh rebase to reduce delta in files. Now delta only in 1 commit and 3 files. |
- Add per group statistics for received messages in percentages Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
0d8fb22
to
de3fac2
Compare
@kylemarcey Please review as well. |
rosbag2_performance/rosbag2_performance_benchmarking/src/result_utils.cpp
Show resolved
Hide resolved
@MichaelOrlov looks good to me, but I cannot approve |
@emersonknapp This PR has been reviewed by @kylemarcey. Although he doesn't have permission for formal approval in the rosbag2. |
Gist: https://gist.githubusercontent.com/MichaelOrlov/bac91eb0919994a67ce69cc69760a6b0/raw/1b520d8f2c93fdac137952b0a34266aea5a364bb/ros2.repos |
@emersonknapp Friendly ping for approval. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I think you should feel free to move forward with rosbag2_performance
PRs without my explicit review. As it's not the released product, as long as it is doing what you want it to, I have no major concerns
@emersonknapp As regrds
I wish I could be able to move forward on my own as you mentioned. But I can't merge without formal approval from maintainers if I am the author of the PR. |
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-2023-09-21/33733/1 |
i.e. counting missed messages from each publisher. Before we were counting only from the first publisher.