Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions urdf_parser/src/joint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ bool parseJointLimits(JointLimits &jl, tinyxml2::XMLElement* config)
{
try {
jl.effort = strToDouble(effort_str);
if (jl.effort < 0.0)
{
CONSOLE_BRIDGE_logError("effort value (%s) is negative", effort_str);
return false;
}
} catch(std::runtime_error &) {
CONSOLE_BRIDGE_logError("effort value (%s) is not a valid float", effort_str);
return false;
Expand All @@ -156,12 +161,80 @@ bool parseJointLimits(JointLimits &jl, tinyxml2::XMLElement* config)
{
try {
jl.velocity = strToDouble(velocity_str);
if (jl.velocity < 0.0)

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

Right now, by default the velocity limit is set to zero, that's the reason I'm only checking for the negative values

{
CONSOLE_BRIDGE_logError("velocity value (%s) is negative", velocity_str);

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

Sure

return false;
}
} catch(std::runtime_error &) {
CONSOLE_BRIDGE_logError("velocity value (%s) is not a valid float", velocity_str);
return false;
}
}

// Get joint acceleration limit
const char* acceleration_str = config->Attribute("acceleration");
if (acceleration_str == NULL){
CONSOLE_BRIDGE_logDebug("urdfdom.joint_limit: no acceleration, using default value");
jl.acceleration = std::numeric_limits<double>::infinity();
}
else
{
try {
jl.acceleration = strToDouble(acceleration_str);
if (jl.acceleration < 0.0)
{
CONSOLE_BRIDGE_logError("acceleration value (%s) is negative", velocity_str);
return false;
}
} catch(std::runtime_error &) {
CONSOLE_BRIDGE_logError("acceleration value (%s) is not a valid float", acceleration_str);
return false;
}
}

// Get joint deceleration limit
const char* deceleration_str = config->Attribute("deceleration");
if (deceleration_str == NULL){
CONSOLE_BRIDGE_logDebug("urdfdom.joint_limit: no deceleration, using acceleration limit");
jl.deceleration = jl.acceleration;
}
else
{
try {
jl.deceleration = strToDouble(deceleration_str);
if (jl.deceleration < 0.0)
{
CONSOLE_BRIDGE_logError("deceleration value (%s) is negative", deceleration_str);
return false;
}
} catch(std::runtime_error &) {
CONSOLE_BRIDGE_logError("deceleration value (%s) is not a valid float", deceleration_str);
return false;
}
}

// Get joint jerk limit
const char* jerk_str = config->Attribute("jerk");
if (jerk_str == NULL){
CONSOLE_BRIDGE_logDebug("urdfdom.joint_limit: no jerk, using default value");
jl.jerk = std::numeric_limits<double>::infinity();
}
else
{
try {
jl.jerk = strToDouble(jerk_str);
if(jl.jerk < 0.0)
{
CONSOLE_BRIDGE_logError("jerk value (%s) is negative", jerk_str);
return false;
}
} catch(std::runtime_error &) {
CONSOLE_BRIDGE_logError("jerk value (%s) is not a valid float", jerk_str);
return false;
}
}

return true;
}

Expand Down
5 changes: 4 additions & 1 deletion urdf_parser/test/urdf_unit_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ TEST(URDF_UNIT_TEST, parse_joint_doubles)
" <child link=\"l2\"/>"
" <origin xyz=\"0 0 0\" rpy=\"0 0 0\"/>"
" <dynamics damping=\"87.098\" friction=\"3.1290\"/>"
" <limit lower=\"12.34\" upper=\"22.999\" effort=\"99.0\" velocity=\"23.0\"/>"
" <limit lower=\"12.34\" upper=\"22.999\" effort=\"99.0\" velocity=\"23.0\" acceleration=\"10.0\" deceleration=\"5.0\" jerk=\"200.0\"/>"
" <safety_controller soft_lower_limit=\"8.765\" soft_upper_limit=\"9.003\" k_position=\"7.0034\" k_velocity=\"9.998\"/>"
" <calibration rising=\"8.654\" falling=\"0.0445\"/>"
" <mimic joint=\"j2\" multiplier=\"9.87\" offset=\"0.098\"/>"
Expand All @@ -204,6 +204,9 @@ TEST(URDF_UNIT_TEST, parse_joint_doubles)
EXPECT_EQ(22.999, urdf->joints_["j1"]->limits->upper);
EXPECT_EQ(99.0, urdf->joints_["j1"]->limits->effort);
EXPECT_EQ(23.0, urdf->joints_["j1"]->limits->velocity);
EXPECT_EQ(10.0, urdf->joints_["j1"]->limits->acceleration);
EXPECT_EQ(5.0, urdf->joints_["j1"]->limits->deceleration);
EXPECT_EQ(200.0, urdf->joints_["j1"]->limits->jerk);

EXPECT_EQ(8.765, urdf->joints_["j1"]->safety->soft_lower_limit);
EXPECT_EQ(9.003, urdf->joints_["j1"]->safety->soft_upper_limit);
Expand Down
3 changes: 3 additions & 0 deletions xsd/urdf.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,9 @@
<xs:attribute name="upper" type="xs:double" default="0" />
<xs:attribute name="effort" type="xs:double" default="0" />
<xs:attribute name="velocity" type="xs:double" default="0" />
<xs:attribute name="acceleration" type="xs:double" default="INF" />
<xs:attribute name="decceleration" type="xs:double" default="INF" />
<xs:attribute name="decceleration" type="xs:double" default="INF" />
Comment on lines +201 to +202
Copy link

@SuperJappie08 SuperJappie08 Apr 29, 2025

Choose a reason for hiding this comment

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

Shouldn't the second decceleration attribute be jerk

Suggested change
<xs:attribute name="decceleration" type="xs:double" default="INF" />
<xs:attribute name="decceleration" type="xs:double" default="INF" />
<xs:attribute name="deceleration" type="xs:double" default="INF" />
<xs:attribute name="jerk" type="xs:double" default="INF" />

EDIT: After further reviewing my comment, I realized deceleration (decceleration) was also spelled incorrectly.

</xs:complexType>

<!-- safety controller node type -->
Expand Down