Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Gazebo Demo for Mecanum Drive Controller #358

Open
wants to merge 9 commits into
base: iron
Choose a base branch
from

Conversation

Pratham-Pandey
Copy link

Added Holonomic Drive(Mecanum Wheel Based) Demo:

Updates:
- Added a 4 wheeled mecanum wheel base cart demo to try out the mecanum drive controller.
- Updated the documentation.
- Added new folder named "mesh" under "gazebo_ros2_control_demos/urdf" for storing mesh.

NOTE:

  • Gazebo is not loading at all when relative path for mesh is used in URDF. For example:
    <mesh filename="package://gazebo_ros2_control_demos/urdf/mesh/holonomic_drive/roller.stl" scale="0.001 0.001 0.001"/>
  • But is loading properly when absolute path is used. For example:
    <mesh filename="file:///home/Desktop/roller.stl" scale="0.001 0.001 0.001"/>

Updates:
- Added a 4 wheeled mecanum wheel base cart demo to try out the mecanum drive controller.
- Updated the documentation.
- Added new folder named "mesh" under "gazebo_ros2_control_demos/urdf" for storing mesh.
Copy link
Collaborator

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

@Pratham-Pandey Thank you for your contribution. Do you mind to open the PR against or retarget this one to master ? then we can backport this to rest of distros.

I'm not completely against adding meshes for the model, Maybe we can keep roller.stl and use a rectangle as the base ? What do you think ?

- Removed mesh file for base of the cart.
- Removed whitespaces.
@Pratham-Pandey
Copy link
Author

PR against or retarget this one to master

What does that means? This pull request is for ROS Iron and i feel it would be better to merge it with Iron branch. Can it be merged with Master Branch? I don't have much idea about git.

I have this pull request as draft because the cart is not loading in Gazebo when relative path for mesh is used(using "package://"). It is working fine with absolute path, though. Do you have any idea how can I get it working with relative path?

I'm not completely against adding meshes for the model, Maybe we can keep roller.stl and use a rectangle as the base ? What do you think ?

Yes it is Good. But having mesh for roller is a must for the cart to behave like a holonomic drive cart. Apart from that, what is the downside of using mesh in this repo? The base mesh dose'nt look that bad 😆

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Copy link
Collaborator

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Hey @Pratham-Pandey I made same changes in this commit https://github.com/ros-controls/gazebo_ros2_control/tree/ahcorde/iron/mecanum

I didn't try the mecanum controller but the robot is there with the mecanum wheels. Do you mind to cherry-pick the commit and try again?

@Pratham-Pandey
Copy link
Author

@ahcorde ok. I will open this PR for review once done.

@Pratham-Pandey Pratham-Pandey marked this pull request as ready for review July 20, 2024 16:21
@Pratham-Pandey
Copy link
Author

Pratham-Pandey commented Jul 20, 2024

@ahcorde The controller is still working after your changes. I have opened the Pull Request.

Question:

  • Any reason for removing rviz from the launch file?
  • Do I need to add license to the launch file like:
# Copyright 2020 Open Source Robotics Foundation, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

Let me know if any other changes are required.

Thanks!

@ahcorde
Copy link
Collaborator

ahcorde commented Jul 22, 2024

@ahcorde The controller is still working after your changes. I have opened the Pull Request.

Question:

* Any reason for removing rviz from the launch file?

* Do I need to add license to the launch file like:
# Copyright 2020 Open Source Robotics Foundation, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

Let me know if any other changes are required.

Thanks!

  • The reason to remove rviz is that I didn't find a rviz configuration file. If you add a configuration file, happy to add it.
  • Yes, please. Add the license at the top of the file

@ahcorde
Copy link
Collaborator

ahcorde commented Jul 22, 2024

This PR requires this controller ros-controls/ros2_controllers#512. PR is still open.

@Pratham-Pandey
Copy link
Author

This PR requires this controller ros-controls/ros2_controllers#512. PR is still open.

The controller PR needs to be merged before this PR could be merged?

  • Have added the License.
  • It is ok without rviz.

@ahcorde
Copy link
Collaborator

ahcorde commented Jul 22, 2024

The controller PR needs to be merged before this PR could be merged?

Yes, It's a requirement, otherwise the example will not work with the debian packages.

@Pratham-Pandey
Copy link
Author

@ahcorde the controller PR has been merged. Have made some changes in config and URDF of this PR. Let me know if any other changes are required and we could get this PR merged.

@christophfroehlich
Copy link
Contributor

@Pratham-Pandey be aware that iron is EOL and we won't release the mecanum controller or gazebo_ros2_control any more to iron. please rebase your PR to rolling/master branch, and we might backport it to humble if the controller will be merged into humble

Copy link
Collaborator

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

do you mind to target rolling ?

@Pratham-Pandey
Copy link
Author

To target rolling, I need to create a separate PR?

@christophfroehlich
Copy link
Contributor

no,you can rebase yours on top of master, force push, and change the target of the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants