-
Notifications
You must be signed in to change notification settings - Fork 938
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
TrajectoryMonitor: zero sampling frequency disables recording #1542
Conversation
moveit_ros/planning/planning_scene_monitor/src/trajectory_monitor.cpp
Outdated
Show resolved
Hide resolved
3f100fb
to
41d2bef
Compare
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.
I've just tested this using the rqt reconfigure plugin:
I need to set the limit to zero in https://github.com/ros-planning/moveit/blob/bc45aecc465daff77efe2b6c4f92f7c972925bbe/moveit_ros/planning/plan_execution/cfg/PlanExecutionDynamicReconfigure.cfg#L7
And then then disabling only works after the first request:
https://github.com/ros-planning/moveit/blob/bc45aecc465daff77efe2b6c4f92f7c972925bbe/moveit_ros/planning/plan_execution/include/moveit/plan_execution/plan_execution.h#L116-L117
Setting the frequency to zero before path was executed silently ignores this frequency...
As the TrajectoryMonitor may be initialized after the reconfigure callback, we need to initialize the sampling frequency with the current value from parameter server.
df47ba7
to
108c97b
Compare
Thanks a lot, @jschleicher, for carefully testing this and revealing those issues. This change looked so simple, but the devil is in the details (as usual). |
Implements #1538 (comment).
@v4hn, should we disable the monitoring by default in
PlanExecution
?