-
Notifications
You must be signed in to change notification settings - Fork 64
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
Merge various fixes and improvements into the humble branch #131
base: rolling
Are you sure you want to change the base?
Merge various fixes and improvements into the humble branch #131
Conversation
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.
profiles are in a different directory, and I think we already have one almost identical to this: scripts/rmw/rmw_fastrtps_cpp/zero-copy-shm.xml
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.
From what I was able to see, these are indeed identical. The potential path I see here to avoid this duplication would be to make the rmw_fastrtps_cpp/setup.sh
to point to the (now) installed version of the xml file (under share/profiles/*
) although OTOH, these also seem to be experimental scripts, so not sure if the change is justified.
Please let me know if you consider the change worthy so I can apply the change and give these scripts a try to confirm there are no regressions.
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.
@mauropasse / @alsora Is there any strong preference for having these profile files installed instead of what we currently have under scripts/rmw ?
I don't think I have enough experience with this to have an informed decision myself.
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.
gentle ping
performance_metrics/include/performance_metrics/events_logger.hpp
Outdated
Show resolved
Hide resolved
@apojomovsky you should:
|
This commit exposes the ability to select between the events-executor from the dedicated iRobot repo, and the events-executor from the experimental namespace of rclcpp.
eeb585d
to
6a0c1d5
Compare
Fix minor leftovers from removing irobot-events-executor
Thanks for the review @mauropasse , @alsora . Your comments were addressed, PTAL |
This PR:
Exposes an extra argument to the irobot-benchmark CLI, so that we can choose whether to use theexecute_timers_separate_thread
constructor argument, consumed by theEventsExecutor
.