wait a second before updating "current" in RViz#291
Conversation
This is a poor-man's-replacement for rhaschke's work that waits for the current robot state. This can be removed after his work is merged. (See #232) Without the additional sleep the automatic update of the start state will pick a point near the end of the executed trajectory instead of the current state. Let's give the monitor a moment to catch up.
davetcoleman
left a comment
There was a problem hiding this comment.
Can we create an issue to remind us to remove it in the future?
|
Makes sense, whoever merges this, should file an issue. |
|
Instead of pull-requesting hacks, you should send me the full test log of your failure. Otherwise I cannot work on this. Doing the sleep in rviz is only one of many places, a sleep would be required... This doesn't solve the problem at all. |
|
On Mon, Oct 17, 2016 at 02:26:51PM -0700, Robert Haschke wrote:
No it does not. But it allows to move robot arms around in rviz without updating the start state each time. @Jntzko is working on getting a MoveIt kinetic setup running to test this over here right today. |
Makes sense. However, getting your (Indigo) log output might help to progress too ;-) |
|
On Tue, Oct 18, 2016 at 06:57:09AM -0700, Robert Haschke wrote:
Working on that. @Jntzko just said he's doing a night shift to get this to you. :-) |
|
I talked to Robert yesterday on the phone and he definitely agrees that the error this request works around exists. Because this improves the user experience quite a bit (without this patch all trajectories generated through moveit without updating the start-state will start from a configuration slightly different from the actual one), and because I tested this on hardware, I'm going to merge. |
This is a poor-man's-replacement for rhaschke's work that waits for the current robot state. This can be removed after his work is merged. (See #232) Without the additional sleep the automatic update of the start state will pick a point near the end of the executed trajectory instead of the current state. Let's give the monitor a moment to catch up.
This is a poor-man's-replacement for rhaschke's work that waits for the current robot state. This can be removed after his work is merged. (See #232) Without the additional sleep the automatic update of the start state will pick a point near the end of the executed trajectory instead of the current state. Let's give the monitor a moment to catch up.
|
@v4hn you were too fast. I started to write this post, was distracted, and now you already merged. As this is a quick hack for the Kinetic release only, I suggest to rebase the PR to kinetic-devel and merge only there. |
|
Sorry, Isaac apparently moved the deadline half a day back because Dave stated everything is ready. I cherry-picked the commit to j/k already and am going to create an issue to revert them when your changes are ready. |
I really appreciate everything you guys are doing / adding / fixing for MoveIt! and as I've made clear I'm a bit frustrated we've been waiting and holding off for months now this release. We've got to stop adding things. p.s. don't forget to create the issue reminder for this |
done in #303 |
This is a poor-man's-replacement for rhaschke's work that waits for the current robot state. This can be removed after his work is merged. (See moveit#232) Without the additional sleep the automatic update of the start state will pick a point near the end of the executed trajectory instead of the current state. Let's give the monitor a moment to catch up.
This is a poor-man's-replacement for rhaschke's work that waits for the current robot state. This can be removed after his work is merged. (See moveit#232) Without the additional sleep the automatic update of the start state will pick a point near the end of the executed trajectory instead of the current state. Let's give the monitor a moment to catch up.
* PSM::waitForCurrentRobotState() + PSM::syncSceneUpdates() * renamed wall_last_state_update_ to last_robot_state_update_wall_time_ * removed PSM::syncSceneUpdates() (and PSM::spinner_, PSM::callback_queue_) Due to an upstream bug, it's not possible to start multiple AsyncSpinners from different threads. Filed PR: ros/ros_comm#867 The spinner is now only needed to serve our own callback_queue_ for scene updates, which is only required for syncSceneUpdates() that syncs all kind of scene updates, not only the robot state. * rviz: execute state update in background ... because we might wait up to 1s for a robot state update * add robot_state update test * waitForRobotToStop() * Revert "wait a second before updating "current" in RViz (#291)" This reverts commit e3ef9a6. * addressed Dave's comments
* PSM::waitForCurrentRobotState() + PSM::syncSceneUpdates() * renamed wall_last_state_update_ to last_robot_state_update_wall_time_ * removed PSM::syncSceneUpdates() (and PSM::spinner_, PSM::callback_queue_) Due to an upstream bug, it's not possible to start multiple AsyncSpinners from different threads. Filed PR: ros/ros_comm#867 The spinner is now only needed to serve our own callback_queue_ for scene updates, which is only required for syncSceneUpdates() that syncs all kind of scene updates, not only the robot state. * rviz: execute state update in background ... because we might wait up to 1s for a robot state update * add robot_state update test * waitForRobotToStop() * Revert "wait a second before updating "current" in RViz (moveit#291)" This reverts commit e3ef9a6. * addressed Dave's comments Conflicts: moveit_ros/planning/planning_scene_monitor/include/moveit/planning_scene_monitor/current_state_monitor.h moveit_ros/planning/planning_scene_monitor/src/planning_scene_monitor.cpp moveit_ros/planning_interface/move_group_interface/src/move_group.cpp moveit_ros/visualization/motion_planning_rviz_plugin/src/motion_planning_frame_planning.cpp
This is a poor-man's-replacement for rhaschke's work that
waits for the current robot state. This can be removed after
his work is merged. (See #232)
Without the additional sleep the automatic update of the start state
will pick a point near the end of the executed trajectory instead of
the current state. Let's give the monitor a moment to catch up.
This is intended to be part of the next release because @rhaschke's code will probably not be in there.