Closed danzimmerman closed 1 year ago
I've only tested this on Humble so far as well, with a complicated installation from source.
There's a workspace snapshot here with deps as submodules: https://github.com/danzimmerman/dz_ws/tree/fb16699a1113cbcd17f6c61724dfb1fe34f96404/src and I'm building against a Robostack Humble binary installation.
I should be able to test against Rolling native apt
binaries shortly.
Worked with Rolling
I see that #54 is going to interact with this and that https://github.com/ros-controls/gz_ros2_control/pull/122 represents movement toward control by joint effort (though probably only for latest versions of Gazebo, not classic, it's a big PR)
Just noticed this:
Is ur_description intended to support other physics engines besides ODE when used in the context of ur_simulation_gazebo?
even though I'm just a user of these packages, based on my experience, "hard-coding" something for a specific simulator in what is -- I believe -- supposed to be a base package with just some robot models, is definitely something to try to avoid.
Ideally, any Gazebo-isms are added in ur_simulation_gazebo
using composition and we avoid violating separation of concerns.
I don't write this as a reviewer though. This is just a comment on your comment/though.
Ideally, any Gazebo-isms are added in
ur_simulation_gazebo
using composition and we avoid violating separation of concerns.I don't write this as a reviewer though. This is just a comment on your comment/though.
Thanks for the comments!
This makes sense to me, and seems to be what #52 is aiming at, without going so far as to split the robot description among several packages.
<dynamics>
tags for Ignition/new Gazebo, and this would be alleviated if we could set simulation-specific properties in simulation-specific code.Unfortunately, I have not yet been successful overriding or adding joint <dynamics>
or any other joint properties with <gazebo reference= />
tags.
Apparently, what's happening is that the SDF conversion adds additional <axis>
, <physics>
and other tags to the SDF joint instead of combining or overriding joint parameters with the info from the <gazebo>
tags.
If I remove shoulder_pan_joint
's URDF <dynamics>
tag and modify the conditional Gazebo additions in ur.urdf.xacro
to this:
I get the following in the SDF, with duplicated <axis>
and <physics>
tags in shoulder pan joint
:
This seems to be a known issue for the <physics>
tag at least:
https://github.com/gazebosim/sdformat/issues/232
and affected other tags in the past
https://github.com/gazebosim/sdformat/issues/71
I'm not seeing anything about duplicated <axis>
tags in the open URDF issues at https://github.com/gazebosim/sdformat/labels/URDF, but that does seem to be what's blocking my ability to use <gazebo>
tags to change joint friction or inject physics tags like <max_force>
.
https://classic.gazebosim.org/tutorials?tut=ros_urdf&cat=connect_ros#%3Cgazebo%3EElementsForJoints says:
<gazebo>
Elements For JointsAgain, similar to <gazebo>
elements for <robot>
and <link>
, any arbitrary blobs that are not parsed according to the table above are inserted into the the corresponding <joint>
element in the SDF. This is particularly useful for plugins, as discussed in the ROS Motor and Sensor Plugins tutorial.
even though I'm just a user of these packages, based on my experience, "hard-coding" something for a specific simulator in what is -- I believe -- supposed to be a base package with just some robot models, is definitely something to try to avoid.
I definitely agree with and appreciate your point here.
Instead of what I'm doing here, I think might propose something like adding generic <joint_name>_damping
and <joint_name>_friction
parameters to ur_macro.xacro
to allow users to pass in appropriate values from a higher-level .xacro
. Those could all default to zero as they do now, for the real robot, FakeSystem
and anything else that does not need to specify them.
Joint dynamics parameters could then be easily specified in any .urdf.xacro
files in ur_simulation_gazebo
, ur_simulation_ignition
, whatever packages are driving #54, and other consumers.
I think any such proposed change should wait for #52 and build on top of it, because that's seems to be a helpful simplifying step toward consuming ur_macro.xacro
in other contexts.
It's not the best or only fix for ur_simulation_gazebo
issue 19, since setting urdf_joint_friction
to a non-zero value is still kind of a hack, but there could be other reasons to modify the URDF joints' <dynamics>
tags arbitrarily from outside of this package.
I've started drafting an alternate approach based on my comment above, so I'm going to close this. Thanks for the comments @gavanderhoorn.
My alternate approach builds on top of the work in progress in #52 and that makes it easier to set package-specific joint damping/friction parameters, so I'll open something for discussion later if that moves forward.
This PR follows on the discussion at https://github.com/UniversalRobots/Universal_Robots_ROS2_Gazebo_Simulation/issues/19
Summary
Simulated UR robots in Gazebo Classic in ROS 2 are not stable as described in the issue above.
This PR stabilizes that behavior (only tested with ODE so far) by adding friction in the joints'
<dynamics>
tags ifsim_gazebo
is true.Details
There are a few discussions of similar issues I've found. In particular gazebo_ros2_control/issues/54 suggests a couple of different approaches:
SetParam()
method in source code to set thefmax
parameter, as suggested here (also suggesting thatjoint_limits_interface
might handle this at some point)In the end, both of these suggestions appear to achieve the same thing for position- and velocity- controlled joints:
fmax
parameter (dParamFMax
in Gazebo source code, see here) was set explicitly by a function call ingazebo_ros_control
here if the joints used position or velocity command interfaces.dParamFMax
, see here. Empirically, that does seem to be what happens when I pass friction through a joint<dynamics>
tag like I do in this PR.In this PR I set the "friction" to 100x the maximum joint effort for reasons suggested here. In short, it seems that
dParamFMax
is a mathematical parameter more than something which should be usually set to a physical joint parameter.The ODE manual says this about joint motors:
This describes a geared motor with no supervision operating at its stall torque, but I wouldn't characterize the UR's joints that way. I believe the robot will fault well before it slows down to limit itself at some max torque.
Limitations
If the UR ROS 2 Gazebo simulation ever evolves toward supporting
EffortJointInterface
like universal_robot/issues/521, then injecting friction through the robot description would appear to violate the guidance in this comment. That said, ifgazebo_ros2_control
eventually handles this, I'd think it'd be better to appropriately reset the joints if possible.Also, an effort command interface is only useful in simulation, since the real robot can't accept joint-level effort commands.
I also haven't tried to test this against the other physics engines yet. I'd currently expect:
JointPtr->SetParam()
is called, and here if it describes<axis>
friction.JointPtr->SetParam()
is called and here if an<axis>
element is parsed.However I don't understand enough about Gazebo and the URDF->SDF->In-Memory Joint pipeline to know if that's where the
<dynamics>
tag ends up.Questions
This PR seems to provide an expedient fix for ur_simulation_gazebo/issues/19 but I'm not sure it's the right approach. I'm wondering:
ur_description
intended to support other physics engines besides ODE when used in the context ofur_simulation_gazebo
?ur_description
or in the simulation packages to support effort joint interfaces like universal_robot/issues/521?