Skip to content
This repository was archived by the owner on Nov 13, 2017. It is now read-only.

Speed up loading of KDL and SRV kinematic plugins#541

Closed
davetcoleman wants to merge 2 commits intomoveit:indigo-develfrom
davetcoleman:_kinematics_base_speedup
Closed

Speed up loading of KDL and SRV kinematic plugins#541
davetcoleman wants to merge 2 commits intomoveit:indigo-develfrom
davetcoleman:_kinematics_base_speedup

Conversation

@davetcoleman
Copy link
Copy Markdown
Member

This needs to be merged at the same time as moveit/moveit_core#216

Improves the KDL and SRV kinematic plugins to load faster by bypassing loading and parsing the robot model.

Changes:

  • Accept robot_model pointer
  • Rename robot_state object
  • Remove need for RDF loader
  • Remove unused robot_state objects
  • Cleanup formatting

…tr() function

Conflicts:
	planning/srv_kinematics_plugin/include/moveit/srv_kinematics_plugin/srv_kinematics_plugin.h
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the old initialization function should either work correctly and print a deprecation warning or should not compile at all. As it is here, it will compile and then not work. Better to fail at compile time than run time.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I was not sure best practice for this. The problem is that we have to keep this function for the IKFast plugins in the wild that we cannot port (without asking users to regenerate them) but for KDL we no longer need this function. I could leave all the old URDF loading code in this plugin to allow this function to still load without having a RobotModel passed to it, but that seems like an overly-complex solution. As long as the new initialize() function is implemented (as it is in this plugin) this old version will never be called unless somewhere else in the API someone calls this deprecated version. If they do, they are using MoveIt! wrong this friendly error message will be displayed. Please explain how I could do this better.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should keep the implementation in the old initialization function and print a deprecation warning there for now.

@sachinchitta
Copy link
Copy Markdown
Contributor

See comment for moveit/moveit_core#216

otamachan pushed a commit to otamachan/moveit_ros that referenced this pull request Oct 22, 2017
* Implements optional ompl_planning config parameter 'force_joint_model_state_space'.

* Renames parameter to 'enforce_joint_model_state_space'.
Expands workaround comment.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants