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

Add per group statistics in rosbag2_performance_benchmarking #1306

Conversation

MichaelOrlov
Copy link
Contributor

@MichaelOrlov MichaelOrlov commented Apr 24, 2023

@MichaelOrlov MichaelOrlov marked this pull request as ready for review April 25, 2023 22:04
@MichaelOrlov MichaelOrlov requested a review from a team as a code owner April 25, 2023 22:04
@MichaelOrlov MichaelOrlov requested review from emersonknapp and jhdcs and removed request for a team April 25, 2023 22:04
@MichaelOrlov
Copy link
Contributor Author

@carlossvg Could you please review this PR?

@MichaelOrlov MichaelOrlov force-pushed the morlov/set_cpu_affinity_in_performance_bechmarking branch from f5474e0 to 4f1bf1b Compare May 30, 2023 21:38
@MichaelOrlov MichaelOrlov force-pushed the morlov/add_per_group_statistics_in_performance_benchmarking branch from 400821a to 0d8fb22 Compare May 30, 2023 21:44
@MichaelOrlov
Copy link
Contributor Author

Note. I've made a fresh rebase to reduce delta in files. Now delta only in 1 commit and 3 files.

@delete-merged-branch delete-merged-branch bot deleted the branch rolling May 31, 2023 02:11
- Add per group statistics for received messages in percentages

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov MichaelOrlov force-pushed the morlov/add_per_group_statistics_in_performance_benchmarking branch from 0d8fb22 to de3fac2 Compare May 31, 2023 02:13
@MichaelOrlov MichaelOrlov changed the base branch from morlov/set_cpu_affinity_in_performance_bechmarking to rolling May 31, 2023 02:13
@MichaelOrlov
Copy link
Contributor Author

@kylemarcey Please review as well.

@kylemarcey
Copy link
Contributor

@MichaelOrlov looks good to me, but I cannot approve

@MichaelOrlov
Copy link
Contributor Author

@emersonknapp This PR has been reviewed by @kylemarcey. Although he doesn't have permission for formal approval in the rosbag2.
Could you please give formal approval or review this PR additionally?

@MichaelOrlov
Copy link
Contributor Author

MichaelOrlov commented Jul 19, 2023

Gist: https://gist.githubusercontent.com/MichaelOrlov/bac91eb0919994a67ce69cc69760a6b0/raw/1b520d8f2c93fdac137952b0a34266aea5a364bb/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_performance_benchmarking
TEST args: --packages-above rosbag2_performance_benchmarking
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/12401

  • Linux Build Status Auto-rerun Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@MichaelOrlov
Copy link
Contributor Author

@emersonknapp Friendly ping for approval.

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.

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

@MichaelOrlov
Copy link
Contributor Author

@emersonknapp As regrds

In general I think you should feel free to move forward with rosbag2_performance PRs without my explicit review.

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.

@MichaelOrlov MichaelOrlov merged commit 7356c9e into rolling Jul 21, 2023
14 checks passed
@delete-merged-branch delete-merged-branch bot deleted the morlov/add_per_group_statistics_in_performance_benchmarking branch July 21, 2023 21:46
@ros-discourse
Copy link

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

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