Adding a missing dependency declaration on joint_limits_interface

[ct2034]

Adding a missing dependency declaration on joint_limits_interface

[gavanderhoorn]

The code seems to be using it, but why does a planner have a direct dependency on something from ros_control which is typically only used by hardware_interface implementations?

[codecov]

Codecov Report

Merging #2487 (19dd302) into master (a9c8f93) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2487      +/-   ##
==========================================
+ Coverage   60.24%   60.24%   +0.01%     
==========================================
  Files         351      351              
  Lines       26477    26477              
==========================================
+ Hits        15948    15949       +1     
+ Misses      10529    10528       -1     

Impacted Files	Coverage Δ
...nning_scene_monitor/src/planning_scene_monitor.cpp	68.32% <0.00%> (+0.16%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9c8f93...19dd302. Read the comment docs.

[martiniil]

The code seems to be using it, but why does a planner have a direct dependency on something from ros_control which is typically only used by hardware_interface implementations?

It allows us to represent joint limits and populate them from the parameter server.

[jschleicher]

@gavanderhoorn What would you suggest? We need the limits for time parametrization of the trajectory. Is there a URDF::JointLimits class or something more appropriate here?

[gavanderhoorn]

We have RobotModel::getVariableBounds(..) fi, which you already appear to be using (here fi).

My comment is not an "official review". I just happened to notice that we're adding a direct dependency on ros_control to a (core) planner of MoveIt, while MoveIt itself also loads joint limits. Using parallel functionality from another package seemed strange to me, hence the question.

[ct2034]

Thanks for the feedback @gavanderhoorn . Would you still agree to merge it like this? I agree that a solution with RobotModel::getVariableBounds(..) would be neater but I would prefer to fix that separately and put it onto our backlog.

[simonschmeisser]

I agree with @gavanderhoorn that this dependency should be killed asap but it should also not be a hidden surprise

[AndyZe]

^ i agree with that logic

[AndyZe]

An issue should probably be created to remove the dependency.

[rhaschke]

Approving and merging. @jschleicher: Could you please work on a PR to replace this interface and use the existing RobotModel interface instead?

[jschleicher]

Thanks for merging. Yes, I've put it on our backlog.