[ROS 2] Fix how wheel properties are retrieved in Ackermann Plugin #1427
Conversation
There was a problem hiding this comment.
The current implementation has implicit assumptions of what the collision mesh of the wheel should be but this is not apparent to users who may spend a lot of time figuring out why their gazebo model is not responding as required.
I agree with this and also think that @siverhavso is right on issue #1239 about a total silent in the presence of errors. The PR looks to me like quite disrupting with the current behavior supported (getting info from meshes) so a more conservative approaches could be:
- Check if kinematic information is provided in the SDF (current pr code) and if not fallback to the old behavior and implement an error message if both fail.
- Try using the current approach of getting info from meshes and fallback to this PR code if GetRadius fails.
- Implement a ROS param to tell the plugin if someone wants the current approach or the new one in this PR.
Not an strong opinion but going with 3 defaulting with current code could help us to keep compatibility and consistency with diff_drive.
| <!-- kinematics --> | ||
| <wheel_radius>0.3</front_left_joint> | ||
| <wheel_base>2.0</wheel_base> | ||
| <track_width>1.2</track_width> |
There was a problem hiding this comment.
Could we please include a comment to indicate that this means "wheel_seperation" in ackermann language? Should help people to understand what really is just from reading.
Signed-off-by: Yadunund <[email protected]>
Signed-off-by: Yadunund <[email protected]>
Signed-off-by: Yadunund <[email protected]>
Signed-off-by: Yadunund <[email protected]>
Yadunund
force-pushed
the
fix_ackermann_wheel_radius
branch
from
32cde1e
to
7410afe
Compare
|
Sorry for the long overdue changes @j-rivero. I refactored the implementation to default to the existing behavior of inferring properties from the collision mesh file. If the radius computation fails, an error is printed to inform the users that we only support collision meshes that are cylinders or spheres. Alternatively, they can set |
| the values of the wheel params below. --> | ||
| <wheel_params_from_collision_mesh>true</wheel_params_from_collision_mesh> | ||
| <!-- The radius of the wheel in meters. Used when wheel_params_from_collision_mesh is false. --> | ||
| <wheel_radius>0.3</front_left_joint> |
There was a problem hiding this comment.
This should be
| <wheel_radius>0.3</front_left_joint> | |
| <wheel_radius>0.3</wheel_radius> |
This PR fixes #1239 by retrieving all required geometric parameters from SDF params as opposed to calculating them from meshes with certain implicit assumptions of mesh geometry. The parameter name
track_widthwas chosen instead ofwheel_separationas this usually nomenclature for automobiles which theackermannplugin will most likely be used with.The current implementation has implicit assumptions of what the collision mesh of the wheel should be but this is not apparent to users who may spend a lot of time figuring out why their gazebo model is not responding as required.
Other plugins in this repo like the diff_drive also gets the parameters from the SDF and not via meshes.
Signed-off-by: Yadunund [email protected]