Skip to content
This repository was archived by the owner on Oct 17, 2025. It is now read-only.

Conversation

@ahcorde
Copy link
Collaborator

@ahcorde ahcorde commented May 28, 2020

I have created this package for testing the gazebo_ros2_control. This package includes:

  • Launch files:
    • Launch Gazebo with a cart in a 30 meter rail
    • load the URDF based on a xacro file with the controllers controllers parameters,
    • robot_state_publisher
  • Config file
    • Config file with the type of controllers
  • Two examples that publish on follow_joint_trayectory

positionController

Signed-off-by: ahcorde [email protected]

Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

Looks great!

Just minor points:

  • Could you drop a few notes about reproducing this into a readme please? Which ROS2 distro, what was compiled from source?

  • Also, could you please decide whether to remove or uncomment the commented code in this PR?

@ahcorde
Copy link
Collaborator Author

ahcorde commented May 28, 2020

Could you drop a few notes about reproducing this into a readme please? Which ROS2 distro, what was compiled from source?

I added in this issue some instructions about how to reproduce it #3

I will update the README.md with more detailed instructions

Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together, this is a great start!

@ahcorde
Copy link
Collaborator Author

ahcorde commented Jun 3, 2020

Error is calculated for revolute joint not for others type of joints (continuous or prismatic). ros-controls/ros2_controllers#54

@chapulina
Copy link
Collaborator

I will update the README.md with more detailed instructions

+1, this would be very helpful. Both a top-level README explaining the packages in this repo, ROS version, etc, as well as a README per-package with quick instructions for each package.

@ahcorde
Copy link
Collaborator Author

ahcorde commented Jun 22, 2020

Added position PID example

gazebo_ros2_control_position_pid

@ahcorde ahcorde self-assigned this Jun 22, 2020
@ahcorde ahcorde added the enhancement New feature or request label Jun 22, 2020
Signed-off-by: ahcorde <[email protected]>
@ahcorde ahcorde mentioned this pull request Jun 22, 2020
Copy link
Collaborator

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

I tried a few demos with the instructions from #12 but they're not working for me

@ahcorde
Copy link
Collaborator Author

ahcorde commented Jun 26, 2020

related to this PR #24

@ahcorde
Copy link
Collaborator Author

ahcorde commented Jul 9, 2020

need a rebase of this PR #28 when is merge on master

Signed-off-by: Louise Poubel <[email protected]>
Copy link
Collaborator

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

CI is passing now, but I think it's because some exec dependencies are not declared.

@ahcorde ahcorde requested a review from chapulina July 24, 2020 08:27
@ahcorde
Copy link
Collaborator Author

ahcorde commented Jul 24, 2020

Pr to fix the CI is upstream ros-controls/ros2_controllers#81

Copy link
Collaborator

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

LGTM. I just have some small comments. I'd also suggest adding a README for this package, and also mentioning it on the top-level README.

params = {'robot_description': robot_desc}

context = LaunchContext()
command = Command('xacro %s -o /tmp/test_cart_position.urdf' % xacro_file)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using the /tmp directory feels a bit fragile. I wouldn't block this PR on this, but we should consider doing this some other way, maybe storing the URDF in a parameter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed a8ebd15

@ahcorde ahcorde merged commit 3b8d572 into master Jul 28, 2020
@ahcorde ahcorde deleted the ahcorde/initial/demos branch July 28, 2020 06:54
@ahcorde ahcorde mentioned this pull request Jul 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants