-
Notifications
You must be signed in to change notification settings - Fork 312
New system plugin called DriveToPoseController
#2679
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
Conversation
Signed-off-by: Saurabh Kamat <[email protected]>
Signed-off-by: Saurabh Kamat <[email protected]>
Signed-off-by: Saurabh Kamat <[email protected]>
Signed-off-by: Saurabh Kamat <[email protected]>
Signed-off-by: Saurabh Kamat <[email protected]>
Signed-off-by: Saurabh Kamat <[email protected]>
Signed-off-by: Saurabh Kamat <[email protected]>
Signed-off-by: Saurabh Kamat <[email protected]>
Signed-off-by: Saurabh Kamat <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution. Here is some feedback.
topicNamespace + "/cmd_pose", &DriveToPoseControllerPrivate::OnCmdPose, | ||
this->dataPtr.get()); | ||
|
||
this->dataPtr->node.Subscribe( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing for subscribers, we can detect if there are publishers. If no publishers inform the user that we need to load the correct set of plugins. (We could also automatically load them if that is something you are willing to look into).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arjo129 Thanks for all the feedback! It makes perfect sense to warn users if no publishers or subscribers are found. I have made the changes to check for the availability of necessary publishers and subscribers and warn users to load the required plugins.
However, I agree it would be a good idea to load the plugins ourselves if they are found to be missing, and I would be happy to implement this. If you have any references for this that would be great help. Also, do you want this change to be a part of this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of the top of my head there is a service to load plugins:
msgs::EntityPlugin_V req; |
Since the system is running inside the server it may be worth exposing an API for doing this via the ECS instead of relying on gz-transport for later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sauk2 can you create an issue to add this functionality in follow up PRs.
Since the system is running inside the server it may be worth exposing an API for doing this via the ECS instead of relying on gz-transport for later.
Yes, we currently have the possibility to load plugins with the LoadSdfPlugins
event (
gz-sim/include/gz/sim/Events.hh
Line 61 in 13bf5be
using LoadSdfPlugins = common::EventT<void(Entity, sdf::Plugins), |
Ideally, we would create a new component that holds the list of plugins attached to an entity. Or, maybe better would be to create entities for each plugin and use the ParentEntity
component to point to the entity they're attached to. The latter would be my preference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created a new issue (#2773) for this discussion.
Signed-off-by: Saurabh Kamat <[email protected]>
Signed-off-by: Saurabh Kamat <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes, otherwise look good to me
…tests Signed-off-by: Saurabh Kamat <[email protected]>
Thank you for the feedback! I have made the requested changes. |
Signed-off-by: Saurabh Kamat <[email protected]>
Tests are failing on windows because DART is missing in our windows CI.
Can you disable these tests on windows? |
Signed-off-by: Saurabh Kamat <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that it works as expected ! Congrats @sauk2
We need to find some ways to give this a bit of publicity. Some ideas:
- Post in Gazebo community forum. You can reuse mainly what you wrote for this PR.
- Write a new PR for adding a tutorial in https://gazebosim.org/api/sim/8/tutorials.html
Thanks Jose, for testing it out. I will work on contributing a tutorial soon! |
Will this be ported to Harmonic at some point? This would provide a nice alternative to nav2 in simple use cases/demos. |
@Mergifyio backport gz-sim8 It was a really nice contribution indeed! |
New system plugin to gz-sim called DriveToPoseController: This controller receives a target pose in the Gazebo coordinate system on /cmd_pose topic and calculates and publishes a command velocity on /cmd_vel topic to move the robot towards the target. --------- Signed-off-by: Saurabh Kamat <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> Co-authored-by: Addisu Z. Taddese <[email protected]>
Stupid bot. @elmeripk could you please check if #2939 works for you on Harmonic? |
🎉 New feature
Closes #2379
Summary
This PR adds a new system plugin to
gz-sim
calledDriveToPoseController
. This is not a standalone plugin and requires other systems plugins (DiffDrive
andOdometryPublisher
) to function.This controller receives a target pose in the Gazebo coordinate system on
/cmd_pose
topic and calculates and publishes a command velocity on/cmd_vel
topic to move the robot towards the target. This plugin contains separate linear and angular proportional controllers with adjustable gains. The user can also specify an allowable deviation from the goal pose in the form of linear and angular deviations. Upon reaching the goal pose, the controller publishes an acknowledgement on/reached_pose
topic to indicate the end of the navigation operation.The plugin needs to be attached to a model entity as in the SDF as follows,
It has the following parameter tags,
<linear_p_gain>
- (Optional) Proportional gain for the linear velocity controller. Default: 1.0<angular_p_gain>
- (Optional) Proportional gain for the angular velocity controller. Default: 2.0<linear_deviation>
- (Optional) Allowable linear deviation (in meters) from the desired coordinate. Default: 0.1<angular_deviation>
- (Optional) Allowable angular deviation (in radians) from the desired orientation. Default: 0.05The following example warehouse world can be found at
src/gz-sim/examples/worlds/drive_to_pose_controller.sdf
with an AMR spawned at its start position.Test it
The integration test for this plugin can be run using the following command,
colcon test --event-handlers console_direct+ --merge-install \ --packages-select gz-sim9 --ctest-args -R drive_to_pose_controller
Use the following steps to test the functionality in the warehouse world by sending the robot to four different poses,
[x, y, orientation] = [1.78, -9.4, -3.14]
[x, y, orientation] = [-3.75, -9.4, 1.57]
[x, y, orientation] = [-3.75, 9.18, 3.14]
[x, y, orientation] = [-5.9, 9.18, 3.14]
The entire operation is shown in the following video,
drive_to_pose_controller_test.mp4
Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.