Skip to content

Full back port of #350 and #298#721

Closed
k-okada wants to merge 3 commits intomoveit:indigo-develfrom
k-okada:full_350
Closed

Full back port of #350 and #298#721
k-okada wants to merge 3 commits intomoveit:indigo-develfrom
k-okada:full_350

Conversation

@k-okada
Copy link
Copy Markdown
Contributor

@k-okada k-okada commented Dec 12, 2017

see #717 (comment) for discussion

Description

Please explain the changes you made, including a reference to the related issue if applicable

Checklist

  • Required: Code is auto formatted using clang-format
  • Extended the tutorials / documentation, if necessary reference
  • Include a screenshot if changing a GUI
  • Optional: Created tests, which fail without this PR reference
  • Optional: Decide if this should be cherry-picked to other current ROS branches (Indigo, Jade, Kinetic)

…State() (moveit#298)

.. to better reflect the actual semantics of the method
to maintain API compatibility, the old function still exists, but is deprecated
Conflicts:
	moveit_ros/planning/planning_scene_monitor/include/moveit/planning_scene_monitor/current_state_monitor.h
	moveit_ros/planning/planning_scene_monitor/src/current_state_monitor.cpp
	moveit_ros/planning_interface/move_group_interface/src/move_group.cpp
* 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
@rhaschke
Copy link
Copy Markdown
Contributor

Thanks for your effort, @k-okada. I will try to review today. However, first I need to discuss the more fundamental question, how to solve the issue at all (by back-porting #350 and breaking ABI or by some other poor-man's approach).

@rhaschke rhaschke self-requested a review December 12, 2017 09:22
@rhaschke
Copy link
Copy Markdown
Contributor

We prefer ABI-compatibility and thus slightly modifying #717.

@rhaschke rhaschke closed this Dec 12, 2017
@k-okada k-okada deleted the full_350 branch January 16, 2018 04:44
JafarAbdi pushed a commit to JafarAbdi/moveit that referenced this pull request Mar 24, 2022
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.

2 participants