Extend parsing of acceleration, deceleration and jerk limits from limit tag
#212
Conversation
|
This pull request has been mentioned on ROS Discourse. There might be relevant details there: https://discourse.ros.org/t/allow-for-more-complex-joints-in-urdf/42234/3 |
|
This pull request has been mentioned on ROS Discourse. There might be relevant details there: https://discourse.ros.org/t/proposal-to-extend-jointlimits-in-urdf/42831/1 |
urdf_parser/src/joint.cpp
Outdated
| const char* deceleration_str = config->Attribute("deceleration"); | ||
| if (deceleration_str == NULL){ | ||
| CONSOLE_BRIDGE_logDebug("urdfdom.joint_limit: no deceleration, using default value"); | ||
| jl.deceleration = std::numeric_limits<double>::infinity(); |
There was a problem hiding this comment.
This is not a full review, but an isolated comment. If the deceleration is unspecified, wouldn't it make more sense to default to the acceleration value?
Unrelated nit: Bracing and spacing style is inconsistent in this changeset.
There was a problem hiding this comment.
You are right. That's a very good point. I'll do the changes soon
…it tag Signed-off-by: Sai Kishor Kothakota <[email protected]>
Signed-off-by: Sai Kishor Kothakota <[email protected]>
Signed-off-by: Sai Kishor Kothakota <[email protected]>
saikishor
force-pushed
the
extend/joint/limits
branch
from
998b548 to
22d0e79
Compare
There was a problem hiding this comment.
Is someone working on enforcing these in ros2_control? Otherwise these new limits will just confuse folks.
(@bmagyar)
| @@ -156,12 +161,80 @@ bool parseJointLimits(JointLimits &jl, tinyxml2::XMLElement* config) | |||
| { | |||
| try { | |||
| jl.velocity = strToDouble(velocity_str); | |||
| if (jl.velocity < 0.0) | |||
| { | |||
| CONSOLE_BRIDGE_logError("velocity value (%s) is negative", velocity_str); | |||
There was a problem hiding this comment.
I would say that it would help parsing the error logs to say velocity limit instead of velocity value but the rest of the fie seems to already be written that way.
| @@ -156,12 +161,80 @@ bool parseJointLimits(JointLimits &jl, tinyxml2::XMLElement* config) | |||
| { | |||
| try { | |||
| jl.velocity = strToDouble(velocity_str); | |||
| if (jl.velocity < 0.0) | |||
There was a problem hiding this comment.
Is a value of zero valid for velocity, acceleration, and jerk limits? Seems to only make sense as a backward way of disabling the joint.
There was a problem hiding this comment.
Right now, by default the velocity limit is set to zero, that's the reason I'm only checking for the negative values
@forrest-rm We already have the first version of the limit enforcement already integrated into the ros2_control (ros-controls/ros2_control#2047). Once this is merged, we will add support for considering the acceleration, deceleration and jerk limits too |
Needs ros/urdfdom_headers#83
This is needed to be able to parse the newly added limits of acceleration, deceleration and jerk, this would be pretty much useful to integrate them into the ros2_control architecture